coreboot-gerrit November 2020

coreboot-gerrit@coreboot.org

1 participants
3243 discussions

Change in coreboot[master]: mb/supermicro/x11-lga1151-series: switch from dev.init to mb_ops.init
by Arthur Heymans (Code Review) 27 Nov '20

27 Nov '20

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48083 ) Change subject: mb/supermicro/x11-lga1151-series: switch from dev.init to mb_ops.init ...................................................................... Patch Set 6: (1 comment) https://review.coreboot.org/c/coreboot/+/48083/5//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/48083/5//COMMIT_MSG@9 PS5, Line 9: Set mainboard_ops.init directly instead of using the indirection via a : mainboard_enable function. > Yeah, I was confused by the names. > > I had a look at how google boards do it. Some use mainboard chip init, others mainboard dev init, so there shouldn't be a problem, since it's only PCRs Yes, it looks like the tree is inconsistent about this. Can you also motivate why it needs to happen earlier? I think having two different functions named init is confusing, so if anyone want to add some form of mainboard init that requires devices to be enumarated, which is generally the case for ramstage hardware init, he would be in trouble. So I'd recommend keeping this in the device_operations init even if it is not strictly needed for pcr stuff. -- To view, visit https://review.coreboot.org/c/coreboot/+/48083 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I192e69a34fa262b38bc40a95fb11c22a4041d0ae Gerrit-Change-Number: 48083 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 27 Nov 2020 20:25:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/supermicro/x11-lga1151-series: switch from dev.init to mb_ops.init
by Michael Niewöhner (Code Review) 27 Nov '20

27 Nov '20

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48083 ) Change subject: mb/supermicro/x11-lga1151-series: switch from dev.init to mb_ops.init ...................................................................... Patch Set 6: (1 comment) https://review.coreboot.org/c/coreboot/+/48083/5//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/48083/5//COMMIT_MSG@9 PS5, Line 9: Set mainboard_ops.init directly instead of using the indirection via a : mainboard_enable function. > Yeah, I was confused by the names. […] btw. isn't bootblock_mainboard_init run in way before any resources are allocated, too? -- To view, visit https://review.coreboot.org/c/coreboot/+/48083 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I192e69a34fa262b38bc40a95fb11c22a4041d0ae Gerrit-Change-Number: 48083 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 27 Nov 2020 20:23:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/supermicro/x11-lga1151-series: switch from dev.init to mb_ops.init
by Michael Niewöhner (Code Review) 27 Nov '20

27 Nov '20

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48083 ) Change subject: mb/supermicro/x11-lga1151-series: switch from dev.init to mb_ops.init ...................................................................... Patch Set 6: (1 comment) https://review.coreboot.org/c/coreboot/+/48083/5//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/48083/5//COMMIT_MSG@9 PS5, Line 9: Set mainboard_ops.init directly instead of using the indirection via a : mainboard_enable function. > So there is a difference between struct device_operations and struct chip_operations. […] Yeah, I was confused by the names. I had a look at how google boards do it. Some use mainboard chip init, others mainboard dev init, so there shouldn't be a problem, since it's only PCRs -- To view, visit https://review.coreboot.org/c/coreboot/+/48083 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I192e69a34fa262b38bc40a95fb11c22a4041d0ae Gerrit-Change-Number: 48083 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 27 Nov 2020 20:18:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/supermicro/x11-lga1151-series: switch from dev.init to mb_ops.init
by Arthur Heymans (Code Review) 27 Nov '20

27 Nov '20

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48083 ) Change subject: mb/supermicro/x11-lga1151-series: switch from dev.init to mb_ops.init ...................................................................... Patch Set 6: (1 comment) https://review.coreboot.org/c/coreboot/+/48083/5//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/48083/5//COMMIT_MSG@9 PS5, Line 9: Set mainboard_ops.init directly instead of using the indirection via a : mainboard_enable function. > huh? this does not change any behaviour. mainboard_ops. […] So there is a difference between struct device_operations and struct chip_operations. Both have an init function which is probably confusing you. The ramstage are as follows: BS_PRE_DEVICE, BS_DEV_INIT_CHIPS, BS_DEV_ENUMERATE, BS_DEV_RESOURCES, BS_DEV_ENABLE, BS_DEV_INIT, BS_POST_DEVICE, BS_OS_RESUME_CHECK, BS_OS_RESUME, BS_WRITE_TABLES, BS_PAYLOAD_LOAD, BS_PAYLOAD_BOOT, So if you place this in the struct chip_operations it will run before any resource allocation. This is why the mainboard code often places init code in the struct device_operations like the code does here (before this change). Now granted, it looks like this mainboard init code only thouches prc, which is likely ok to do, as a general rule you want that to happen after resource allocation. -- To view, visit https://review.coreboot.org/c/coreboot/+/48083 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I192e69a34fa262b38bc40a95fb11c22a4041d0ae Gerrit-Change-Number: 48083 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 27 Nov 2020 20:13:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/supermicro/x11-lga1151-series: switch from dev.init to mb_ops.init
by Michael Niewöhner (Code Review) 27 Nov '20

27 Nov '20

Hello Felix Singer, build bot (Jenkins), Paul Menzel, Tim Wawrzynczak, Arthur Heymans, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/48083 to look at the new patch set (#6). Change subject: mb/supermicro/x11-lga1151-series: switch from dev.init to mb_ops.init ...................................................................... mb/supermicro/x11-lga1151-series: switch from dev.init to mb_ops.init Use mainboard_ops.init for GPIO configuration instead of using the indirection via a mainboard_enable function. Tested successfully on X11SSM-F. Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de> Change-Id: I192e69a34fa262b38bc40a95fb11c22a4041d0ae --- M src/mainboard/supermicro/x11-lga1151-series/include/mainboard.h M src/mainboard/supermicro/x11-lga1151-series/mainboard.c M src/mainboard/supermicro/x11-lga1151-series/variants/x11ssm-f/mainboard.c 3 files changed, 6 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/48083/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/48083 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I192e69a34fa262b38bc40a95fb11c22a4041d0ae Gerrit-Change-Number: 48083 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/supermicro/x11-lga1151-series: set mainboard_ops.init directly
by Michael Niewöhner (Code Review) 27 Nov '20

27 Nov '20

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48083 ) Change subject: mb/supermicro/x11-lga1151-series: set mainboard_ops.init directly ...................................................................... Patch Set 5: (1 comment) https://review.coreboot.org/c/coreboot/+/48083/5//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/48083/5//COMMIT_MSG@9 PS5, Line 9: Set mainboard_ops.init directly instead of using the indirection via a : mainboard_enable function. > huh? this does not change any behaviour. mainboard_ops. […] ah, you're indeed right about chip.init being run before DEV.init. Anyway, I don't see any problem here, since we only run gpio config. see CB:48086. the rest gets dropped in CB:48092 and CB:48084. -- To view, visit https://review.coreboot.org/c/coreboot/+/48083 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I192e69a34fa262b38bc40a95fb11c22a4041d0ae Gerrit-Change-Number: 48083 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 27 Nov 2020 20:08:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/supermicro/x11-lga1151-series: set mainboard_ops.init directly
by Michael Niewöhner (Code Review) 27 Nov '20

27 Nov '20

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48083 ) Change subject: mb/supermicro/x11-lga1151-series: set mainboard_ops.init directly ...................................................................... Patch Set 5: (1 comment) https://review.coreboot.org/c/coreboot/+/48083/5//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/48083/5//COMMIT_MSG@9 PS5, Line 9: Set mainboard_ops.init directly instead of using the indirection via a : mainboard_enable function. > chip.init is run significantly earlier than dev.init, before resources are allocated. […] huh? this does not change any behaviour. mainboard_ops.init was set by mainboard_enable, which runs before .init . now it's set in the struct at compile time, like it's done for any other chip/driver (except a few, where it's done wrong like here), so where's the problem? -- To view, visit https://review.coreboot.org/c/coreboot/+/48083 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I192e69a34fa262b38bc40a95fb11c22a4041d0ae Gerrit-Change-Number: 48083 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 27 Nov 2020 19:57:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/supermicro/x11-lga1151-series: rework gpio setup to not use headers
by Felix Singer (Code Review) 27 Nov '20

27 Nov '20

Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48087 ) Change subject: mb/supermicro/x11-lga1151-series: rework gpio setup to not use headers ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/48087 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic62ce4335af605c081ef288e892441585ff2bd3e Gerrit-Change-Number: 48087 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> 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 Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 27 Nov 2020 18:52:44 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/supermicro/x11-lga1151-series: configure gpios in mainboard init
by Felix Singer (Code Review) 27 Nov '20

27 Nov '20

Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48086 ) Change subject: mb/supermicro/x11-lga1151-series: configure gpios in mainboard init ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/48086 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If2a54c75c5243d94cdc025c597ee347820b35d32 Gerrit-Change-Number: 48086 Gerrit-PatchSet: 6 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> 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 Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 27 Nov 2020 18:51:01 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/supermicro/x11-lga1151-series: restructure and clean up devicetree
by Felix Singer (Code Review) 27 Nov '20

27 Nov '20

Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48082 ) Change subject: mb/supermicro/x11-lga1151-series: restructure and clean up devicetree ...................................................................... Patch Set 4: Code-Review+1 Please check if the binary is identical -- To view, visit https://review.coreboot.org/c/coreboot/+/48082 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9285d786e973621a732e2627c734adc930e54207 Gerrit-Change-Number: 48082 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 27 Nov 2020 18:45:17 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2020