coreboot-gerrit May 2022

coreboot-gerrit@coreboot.org

1 participants
6166 discussions

Change in coreboot[master]: nb/amd/common: Move RAM calcualtion to common code
by Kyösti Mälkki (Code Review) 03 May '22

03 May '22

Attention is currently required from: Nico Huber, Michał Żygowski, Michał Kopeć, Paul Menzel, Angel Pons, Arthur Heymans, Krystian Hebel. Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61682 ) Change subject: nb/amd/common: Move RAM calcualtion to common code ...................................................................... Patch Set 6: (5 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/61682/comment/32b6fa84_b273c47b PS6, Line 7: nb/amd/common: Move RAM calcualtion to common code calculation File src/northbridge/amd/agesa/family14/northbridge.c: https://review.coreboot.org/c/coreboot/+/61682/comment/908ec2f2_6726f3f9 PS6, Line 27: static unsigned int fx_devs = 0; > I'd simplify the fx_devs stuff first. There is no perfect order. Addressing fx_devs arrays soon extends to no longer passing nodeid and at the end of the day, majority of the code review ends up with code removals like in CB:52934. https://review.coreboot.org/c/coreboot/+/61682/comment/80a06a93_aa220430 PS6, Line 27: static unsigned int fx_devs = 0; > I'd simplify the fx_devs stuff first. Ack https://review.coreboot.org/c/coreboot/+/61682/comment/8fd08925_ace9250e PS6, Line 100: dev = __f1_dev[i]; > dev = pcidev_on_root(DEV_CDB, 1) ? Ack File src/northbridge/amd/common/ram_calc.c: https://review.coreboot.org/c/coreboot/+/61682/comment/2855a570_41d3878d PS5, Line 28: > > No, this is correct, and comment in line #25 is wrong. […] Ack -- To view, visit https://review.coreboot.org/c/coreboot/+/61682 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3d222f6cbd26fe78d968d2883984f25e8b62ab5c Gerrit-Change-Number: 61682 Gerrit-PatchSet: 6 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.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: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Comment-Date: Tue, 03 May 2022 09:10:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Michał Kopeć <michal.kopec(a)3mdeb.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/phy/m88e1512: Add new driver for Marvell PHY 88E1512
by Werner Zeh (Code Review) 03 May '22

03 May '22

Attention is currently required from: Paul Menzel, Mario Scheithauer. Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/64024 ) Change subject: drivers/phy/m88e1512: Add new driver for Marvell PHY 88E1512 ...................................................................... Patch Set 1: (1 comment) File src/soc/intel/elkhartlake/tsn_gbe.c: https://review.coreboot.org/c/coreboot/+/64024/comment/17e62108_f80c6a2a PS1, Line 124: .ops_pci = &pci_dev_ops_pci, > Should this be a separate patch, or at least mentioned in the commit message? Is mentioned in the commit message: "To be able to use the driver, it is necessary to activate the scan bus operation in the corresponding SOC TSN GbE driver." -- To view, visit https://review.coreboot.org/c/coreboot/+/64024 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iabe1aef2217dfaee4b5a4bd83782ab588d4be642 Gerrit-Change-Number: 64024 Gerrit-PatchSet: 1 Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Comment-Date: Tue, 03 May 2022 09:09:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Add SBOM (Software Bill of Materials) Generation
by Maximilian Brune (Code Review) 03 May '22

03 May '22

Attention is currently required from: Nico Huber, Martin L Roth, Angel Pons, Arthur Heymans. Maximilian Brune has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63639 ) Change subject: Add SBOM (Software Bill of Materials) Generation ...................................................................... Patch Set 14: (1 comment) Patchset: PS12: > Given that this change is WIP, I imagine that SBOM entries for other firmware ingredients will be im […] Yes, the goal is that the software owners of these various blobs, supply a SBOM file along with it. So we only have to include it in the build (with a given path). for microcode I am currently extracting the information myself (which is not the intention/goal), but should be good enough for a proof of concept. I am planning to add an option to the goswid tool. It will probably look like this: goswid -o sbom-new.uswid -a my-custom-payload.json sbom-old.uswid the '-a' would be for 'append' Then you only have to use the cbfstool to add sbom-new.uswid into the CBFS. -- To view, visit https://review.coreboot.org/c/coreboot/+/63639 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Icb7481d4903f95d200eddbfed7728fbec51819d0 Gerrit-Change-Number: 63639 Gerrit-PatchSet: 14 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Martin L Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Christian Walter <christian.walter(a)9elements.com> Gerrit-CC: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Martin L Roth <martinroth(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Tue, 03 May 2022 09:03:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/phy/m88e1512: Add new driver for Marvell PHY 88E1512
by Paul Menzel (Code Review) 03 May '22

03 May '22

Attention is currently required from: Mario Scheithauer. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/64024 ) Change subject: drivers/phy/m88e1512: Add new driver for Marvell PHY 88E1512 ...................................................................... Patch Set 1: (4 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/64024/comment/f82276d3_1899554d PS1, Line 14: One can change parameters in device tree so that the used setup can be Plesae add a blank line above. https://review.coreboot.org/c/coreboot/+/64024/comment/b4354fc3_4c056a9f PS1, Line 19: https://www.marvell.com/content/dam/marvell/en/public-collateral/ : transceivers/marvell-phys-transceivers-alaska-88e151x-datasheet.pdf Can be in one line. File src/drivers/phy/m88e1512/chip.h: https://review.coreboot.org/c/coreboot/+/64024/comment/2fccb879_92d76495 PS1, Line 4: uint8_t device_index; Use native types? File src/soc/intel/elkhartlake/tsn_gbe.c: https://review.coreboot.org/c/coreboot/+/64024/comment/cdac6881_7dd713a5 PS1, Line 124: .ops_pci = &pci_dev_ops_pci, Should this be a separate patch, or at least mentioned in the commit message? -- To view, visit https://review.coreboot.org/c/coreboot/+/64024 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iabe1aef2217dfaee4b5a4bd83782ab588d4be642 Gerrit-Change-Number: 64024 Gerrit-PatchSet: 1 Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Comment-Date: Tue, 03 May 2022 08:59:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/ehl: Provide function to change PHY-to-MAC IRQ polarity
by Werner Zeh (Code Review) 03 May '22

03 May '22

Attention is currently required from: Mario Scheithauer. Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63888 ) Change subject: soc/intel/ehl: Provide function to change PHY-to-MAC IRQ polarity ...................................................................... Patch Set 2: (2 comments) File src/soc/intel/elkhartlake/tsn_gbe.c: https://review.coreboot.org/c/coreboot/+/63888/comment/63edd610_0e553fbe PS2, Line 38: udelay(TSN_GMII_DELAY_US); Do you need this udelay() here at all? Are there restrictions in how often you can read the busy bit? If not we could get rind of it completely, stopwatch will handle the rest. https://review.coreboot.org/c/coreboot/+/63888/comment/cb4b20ba_ba71dc9b PS2, Line 44: Timeout at %ld Maybe: "Timeout after %ld msec" -- To view, visit https://review.coreboot.org/c/coreboot/+/63888 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia314014c7cacbeb72629c773c8c0bb5f002a3f54 Gerrit-Change-Number: 63888 Gerrit-PatchSet: 2 Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Comment-Date: Tue, 03 May 2022 08:59:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/ehl: Provide function to change PHY-to-MAC IRQ polarity
by Paul Menzel (Code Review) 03 May '22

03 May '22

Attention is currently required from: Mario Scheithauer, Werner Zeh. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63888 ) Change subject: soc/intel/ehl: Provide function to change PHY-to-MAC IRQ polarity ...................................................................... Patch Set 2: (1 comment) File src/soc/intel/elkhartlake/Kconfig: https://review.coreboot.org/c/coreboot/+/63888/comment/8f40231c_62f12dc1 PS2, Line 246: config EHL_TSN_PHY2MAC_IRQ_ACTIVE_HIGH Instead of Kconfig could that be made a devicetree option? -- To view, visit https://review.coreboot.org/c/coreboot/+/63888 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia314014c7cacbeb72629c773c8c0bb5f002a3f54 Gerrit-Change-Number: 63888 Gerrit-PatchSet: 2 Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Comment-Date: Tue, 03 May 2022 08:56:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/elkhartlake: Provide ability to update TSN GbE MAC addresses
by Paul Menzel (Code Review) 03 May '22

03 May '22

Attention is currently required from: Mario Scheithauer. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63863 ) Change subject: soc/intel/elkhartlake: Provide ability to update TSN GbE MAC addresses ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/63863/comment/c1ee940b_566e4d55 PS3, Line 13: following patch next patch in the series -- To view, visit https://review.coreboot.org/c/coreboot/+/63863 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2303a64cfd09fa02734ca9452d26591af2a76221 Gerrit-Change-Number: 63863 Gerrit-PatchSet: 3 Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Comment-Date: Tue, 03 May 2022 08:55:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/elkhartlake: Implement TSN GbE driver
by Paul Menzel (Code Review) 03 May '22

03 May '22

Attention is currently required from: Mario Scheithauer. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63861 ) Change subject: soc/intel/elkhartlake: Implement TSN GbE driver ...................................................................... Patch Set 3: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/63861/comment/14a48615_5b8293ec PS3, Line 7: TSN Sorry, what is TSN? File src/soc/intel/elkhartlake/tsn_gbe.c: https://review.coreboot.org/c/coreboot/+/63861/comment/a1b8a241_de4f7c8f PS3, Line 19: 0x4b32 Add first to sort? https://review.coreboot.org/c/coreboot/+/63861/comment/e74b82d3_86b76beb PS3, Line 24: .devices = gbe_tsn_device_ids, Either align or do not align the =. -- To view, visit https://review.coreboot.org/c/coreboot/+/63861 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7522914c56b74486bb088280d2686acf7027d1d3 Gerrit-Change-Number: 63861 Gerrit-PatchSet: 3 Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Comment-Date: Tue, 03 May 2022 08:54:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/siemens/mc_ehl2: Adjust PSE TSN settings in devicetree
by Paul Menzel (Code Review) 03 May '22

03 May '22

Attention is currently required from: Mario Scheithauer. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63862 ) Change subject: mb/siemens/mc_ehl2: Adjust PSE TSN settings in devicetree ...................................................................... Patch Set 3: (4 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/63862/comment/7dca5204_b84cabaa PS3, Line 8: What is the current problem? nNot all controllers showing up? https://review.coreboot.org/c/coreboot/+/63862/comment/ad897281_5dcb523d PS3, Line 12: This patch enables the Serial Gigabit Media Independent Interface Please add a blank line above to separate paragraphs. https://review.coreboot.org/c/coreboot/+/63862/comment/a84b6601_c14ee9ad PS3, Line 15: And furthermore, the PCH TSN Link speed is set to 1 : Gbps. Please do that in a separate commit. https://review.coreboot.org/c/coreboot/+/63862/comment/31924fa9_2ed0c560 PS3, Line 19: ifconfig New tools are `ip`. ;-) For example: `ip l` and `ip a`. -- To view, visit https://review.coreboot.org/c/coreboot/+/63862 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I74e660548b2c44d5dbdb6023d5a36cfdd7e96f43 Gerrit-Change-Number: 63862 Gerrit-PatchSet: 3 Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Comment-Date: Tue, 03 May 2022 08:53:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/elkhartlake: Provide ability to update TSN GbE MAC addresses
by Werner Zeh (Code Review) 03 May '22

03 May '22

Attention is currently required from: Mario Scheithauer. Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63863 ) Change subject: soc/intel/elkhartlake: Provide ability to update TSN GbE MAC addresses ...................................................................... Patch Set 3: Code-Review+1 (1 comment) File src/soc/intel/elkhartlake/tsn_gbe.c: https://review.coreboot.org/c/coreboot/+/63863/comment/ca903e58_59dd13b8 PS2, Line 25: *mac_p = (*mac_p & 0xFFFF0000) | (adr_to_set[5] << 8) | adr_to_set[4]; > Great idea. […] Looks more readble IMHO now, thanks. -- To view, visit https://review.coreboot.org/c/coreboot/+/63863 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2303a64cfd09fa02734ca9452d26591af2a76221 Gerrit-Change-Number: 63863 Gerrit-PatchSet: 3 Gerrit-Owner: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Mario Scheithauer <mario.scheithauer(a)siemens.com> Gerrit-Comment-Date: Tue, 03 May 2022 08:50:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Mario Scheithauer <mario.scheithauer(a)siemens.com> Comment-In-Reply-To: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2022