coreboot-gerrit November 2019

coreboot-gerrit@coreboot.org

1 participants
2418 discussions

Change in coreboot[master]: mb/supermicro/x11-lga1151-series: use new console delay Kconfig option
by Michael Niewöhner (Code Review) 03 Nov '19

03 Nov '19

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36592 ) Change subject: mb/supermicro/x11-lga1151-series: use new console delay Kconfig option ...................................................................... Uploaded patch set 5. -- To view, visit https://review.coreboot.org/c/coreboot/+/36592 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8bf4ef7ad9beea7b3dc22e1567623a423597eff9 Gerrit-Change-Number: 36592 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Michael Niewöhner Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 03 Nov 2019 17:17:44 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/supermicro/x11-lga1151-series: use new console delay Kconfig option
by Michael Niewöhner (Code Review) 03 Nov '19

03 Nov '19

Hello Patrick Rudolph, Matt DeVillier, build bot (Jenkins), Nico Huber, Patrick Georgi, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/36592 to look at the new patch set (#5). Change subject: mb/supermicro/x11-lga1151-series: use new console delay Kconfig option ...................................................................... mb/supermicro/x11-lga1151-series: use new console delay Kconfig option This replaces the hardcoded delay by the new Kconfig option. Change-Id: I8bf4ef7ad9beea7b3dc22e1567623a423597eff9 Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de> --- M src/mainboard/supermicro/x11-lga1151-series/Kconfig M src/mainboard/supermicro/x11-lga1151-series/bootblock.c 2 files changed, 2 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/36592/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/36592 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8bf4ef7ad9beea7b3dc22e1567623a423597eff9 Gerrit-Change-Number: 36592 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Michael Niewöhner Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: superio/aspeed/common: add workaround for serial routing delay quirk
by Michael Niewöhner (Code Review) 03 Nov '19

03 Nov '19

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36591 ) Change subject: superio/aspeed/common: add workaround for serial routing delay quirk ...................................................................... Uploaded patch set 5. -- To view, visit https://review.coreboot.org/c/coreboot/+/36591 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7d6599b76384fc94a00a9cfc1794ebfe34863ff9 Gerrit-Change-Number: 36591 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jacob Garber <jgarber1(a)ualberta.ca> 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: Michael Niewöhner Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 03 Nov 2019 17:17:44 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: superio/aspeed/common: add workaround for serial routing delay quirk
by Michael Niewöhner (Code Review) 03 Nov '19

03 Nov '19

Hello Kyösti Mälkki, Patrick Rudolph, Arthur Heymans, Jacob Garber, Matt DeVillier, build bot (Jenkins), Nico Huber, Martin Roth, Patrick Georgi, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/36591 to look at the new patch set (#5). Change subject: superio/aspeed/common: add workaround for serial routing delay quirk ...................................................................... superio/aspeed/common: add workaround for serial routing delay quirk Some mainboards with an ASPEED BMC do the serial routing setup in the BMC boot phase on cold boot. This results in scrambled console output when this is not finished fast enough. This adds a delay of 500ms as workaround in the BMCs uart setup that can be selected at mainboard level. A user may disable the workaround when using another BMC firmware like OpenBMC, u-bmc or some custom BMC bootloader with fast serial setup. Change-Id: I7d6599b76384fc94a00a9cfc1794ebfe34863ff9 Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de> --- M src/superio/aspeed/common/Kconfig M src/superio/aspeed/common/early_serial.c 2 files changed, 20 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/36591/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/36591 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7d6599b76384fc94a00a9cfc1794ebfe34863ff9 Gerrit-Change-Number: 36591 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Jacob Garber <jgarber1(a)ualberta.ca> 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: Michael Niewöhner Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: console: serial: add optional delay to wait for BMC serial routing
by Arthur Heymans (Code Review) 03 Nov '19

03 Nov '19

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36591 ) Change subject: console: serial: add optional delay to wait for BMC serial routing ...................................................................... Patch Set 4: > Patch Set 4: > > > Patch Set 4: > > > > > Patch Set 4: > > > > > > > Patch Set 4: > > > > > > > > > Patch Set 4: > > > > > > > > > > I'm not really convinced that adding quirk handling in generic common code like this is such a good idea. What is wrong with the current code which adds a delay in the bootblock in the mainboard code? > > > > > > > > Because not only one board has that problem > > > > > > The mainboard bootblock code is always supposed to setup the superio at the moment so handling the quirks there is the best place to do that. It's ok to have some duplication of that. Exposing the amounts of seconds in Kconfig is not really nice, but that's a personal opinion. I don't like Kconfig options to be something you have to tune and experiment with to get it right. It should present just options that are meaningful for different use cases. > > > > > I don't like Kconfig options to be something you have to tune and experiment with to get it right. > > > > You don't have to. The default value which works is set by the board; see follow-up commit. When the user flashes a fixed BMC he can simply set it to 0 or any other value that fits then. > > > > Having a hard-coded delay is not what we would want. This CB makes it dynamic by checking how much time is needed to wait. The mb approach *always* delays 1s. > > If we don't want to have a Kconfig for the seconds, we could set in in devicetree and make the CONFIG_BUG_.... user selectable That sounds ok. Devicetree is a bit silly however, as that is supposed to be a board specific configuration data for other code. If the console and quirk handling happen in the mainboard code as they should be, there is no reason to use the devicetree. -- To view, visit https://review.coreboot.org/c/coreboot/+/36591 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7d6599b76384fc94a00a9cfc1794ebfe34863ff9 Gerrit-Change-Number: 36591 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Jacob Garber <jgarber1(a)ualberta.ca> 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: Michael Niewöhner Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 03 Nov 2019 15:39:36 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: console: serial: add optional delay to wait for BMC serial routing
by Michael Niewöhner (Code Review) 03 Nov '19

03 Nov '19

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36591 ) Change subject: console: serial: add optional delay to wait for BMC serial routing ...................................................................... Patch Set 4: > Patch Set 4: > > > Patch Set 4: > > > > > Patch Set 4: > > > > > > > Patch Set 4: > > > > > > > > I'm not really convinced that adding quirk handling in generic common code like this is such a good idea. What is wrong with the current code which adds a delay in the bootblock in the mainboard code? > > > > > > Because not only one board has that problem > > > > The mainboard bootblock code is always supposed to setup the superio at the moment so handling the quirks there is the best place to do that. It's ok to have some duplication of that. Exposing the amounts of seconds in Kconfig is not really nice, but that's a personal opinion. I don't like Kconfig options to be something you have to tune and experiment with to get it right. It should present just options that are meaningful for different use cases. > > > I don't like Kconfig options to be something you have to tune and experiment with to get it right. > > You don't have to. The default value which works is set by the board; see follow-up commit. When the user flashes a fixed BMC he can simply set it to 0 or any other value that fits then. > > Having a hard-coded delay is not what we would want. This CB makes it dynamic by checking how much time is needed to wait. The mb approach *always* delays 1s. If we don't want to have a Kconfig for the seconds, we could set in in devicetree and make the CONFIG_BUG_.... user selectable -- To view, visit https://review.coreboot.org/c/coreboot/+/36591 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7d6599b76384fc94a00a9cfc1794ebfe34863ff9 Gerrit-Change-Number: 36591 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Jacob Garber <jgarber1(a)ualberta.ca> 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: Michael Niewöhner Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 03 Nov 2019 15:24:05 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: console: serial: add optional delay to wait for BMC serial routing
by Michael Niewöhner (Code Review) 03 Nov '19

03 Nov '19

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36591 ) Change subject: console: serial: add optional delay to wait for BMC serial routing ...................................................................... Patch Set 4: > Patch Set 4: > > > Patch Set 4: > > > > > Patch Set 4: > > > > > > I'm not really convinced that adding quirk handling in generic common code like this is such a good idea. What is wrong with the current code which adds a delay in the bootblock in the mainboard code? > > > > Because not only one board has that problem > > The mainboard bootblock code is always supposed to setup the superio at the moment so handling the quirks there is the best place to do that. It's ok to have some duplication of that. Exposing the amounts of seconds in Kconfig is not really nice, but that's a personal opinion. I don't like Kconfig options to be something you have to tune and experiment with to get it right. It should present just options that are meaningful for different use cases. > I don't like Kconfig options to be something you have to tune and experiment with to get it right. You don't have to. The default value which works is set by the board; see follow-up commit. When the user flashes a fixed BMC he can simply set it to 0 or any other value that fits then. Having a hard-coded delay is not what we would want. This CB makes it dynamic by checking how much time is needed to wait. The mb approach *always* delays 1s. -- To view, visit https://review.coreboot.org/c/coreboot/+/36591 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7d6599b76384fc94a00a9cfc1794ebfe34863ff9 Gerrit-Change-Number: 36591 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Niewöhner Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Jacob Garber <jgarber1(a)ualberta.ca> 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: Michael Niewöhner Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sun, 03 Nov 2019 15:22:20 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: console: serial: add optional delay to wait for BMC serial routing
by Arthur Heymans (Code Review) 03 Nov '19

03 Nov '19

1 0

Change in coreboot[master]: console: serial: add optional delay to wait for BMC serial routing
by Michael Niewöhner (Code Review) 03 Nov '19

03 Nov '19

1 0

Change in coreboot[master]: console: serial: add optional delay to wait for BMC serial routing
by Michael Niewöhner (Code Review) 03 Nov '19

03 Nov '19

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2019