Attention is currently required from: Martin Roth, Piotr Król, Felix Held.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52576 )
Change subject: Documentation: Add suggestion to document flag days
......................................................................
Patch Set 1:
(1 comment)
File Documentation/getting_started/gerrit_guidelines.md:
https://review.coreboot.org/c/coreboot/+/52576/comment/3d56d7bd_7f0ba299
PS1, Line 332: requires a change in all mainboards using it) needs to be documented.
> So, I misunderstood that. I agree with selecting stable point and integrating on top of that, but what should happen when you shipped product and once in a while (e.g. 1x/year) you getting request about fixing something?
You should backport the fix, I think. That's what we always do for Chrome OS. Thankfully fixes tend to be rather small and uncomplicated to backport most of the time.
> Finally I didn't mentioned that, but coreboot does not build production firmware images, it is more like framework then something building images that can go to market. There is always some customer-specific diff that will not be merged upstream. That diff have to be maintained by someone (e.g. for PC Engines it is now ~4k SLOC), bigger difference more expensive maintanance.
I think this is your real problem tbh, and it's one of your own making. Why isn't your code upstream? Google had a downstream fork of coreboot for years, and we suffered immensely under the constant back and forward porting effort just like you describe here, until we eventually learned our lesson forced everything to be landed upstream first before we would ship it. I don't think that this is any "10k+ employees vs small company" difference, the effort is still the same, and the answer is just that working directly upstream is the only way to prevent all that churn. (Google may have 10k+ employees but they're not all working on coreboot, btw. It's more like 20 or so.)
--
To view, visit https://review.coreboot.org/c/coreboot/+/52576
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic7c01a0f1008821ebbda43a426ba1e3d6410e861
Gerrit-Change-Number: 52576
Gerrit-PatchSet: 1
Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 05 May 2021 23:48:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Patrick Georgi <pgeorgi(a)google.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Comment-In-Reply-To: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Martin Roth, Angel Pons.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/51133 )
Change subject: Add support to switch LSPCON modes
......................................................................
Patch Set 11: Code-Review+1
(1 comment)
Patchset:
PS11:
patch train verified on a Purism Librem Mini v2 using HDMI 2.0 output
--
To view, visit https://review.coreboot.org/c/libgfxinit/+/51133
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: libgfxinit
Gerrit-Branch: master
Gerrit-Change-Id: Idfa9bdff47a6591000cd5092d64a4cd4f8dbdc8d
Gerrit-Change-Number: 51133
Gerrit-PatchSet: 11
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Wed, 05 May 2021 23:45:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Martin Roth, Julius Werner, Felix Held.
Piotr Król has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52576 )
Change subject: Documentation: Add suggestion to document flag days
......................................................................
Patch Set 1:
(1 comment)
File Documentation/getting_started/gerrit_guidelines.md:
https://review.coreboot.org/c/coreboot/+/52576/comment/8ef235f6_8a663929
PS1, Line 332: requires a change in all mainboards using it) needs to be documented.
> Well, for Chrome OS firmware branches specifically, the kind of broad API changes we're talking about here usually *shouldn't* be backported. That's kinda what I was trying to say in my email as well, if you're trying to stabilize for a product you shouldn't *want* to keep pulling in every new hot-off-the-presses development from master. You should sit back and only take bug fixes relevant to your platform.
So, I misunderstood that. I agree with selecting stable point and integrating on top of that, but what should happen when you shipped product and once in a while (e.g. 1x/year) you getting request about fixing something?
Of course there should be budget to maintain code up to that point, or pay enough for significant backport. Right now we evaluate effort, if we backport or somehow hack similar fix on branch. Sometimes it happen that customer decide to stay with old code, it maybe doesn't matter much for 10k+ employees corporation, but for 3mdeb it is smaller market. It is also bad marketing for coreboot since OEMs spread the word about costs of coreboot maintenance and choose alternatives.
There is also other perspective, where customer may want some release since end users push them for that. So customer didn't paid for maintenance for some time and we have to deal with massive rebase. At one point rebase will cost N, on other 2x N - in that way it is harder to provider stable maintainership services and convince customers to keep upstreaming their code and provide newer version of firmware.
Finally I didn't mentioned that, but coreboot does not build production firmware images, it is more like framework then something building images that can go to market. There is always some customer-specific diff that will not be merged upstream. That diff have to be maintained by someone (e.g. for PC Engines it is now ~4k SLOC), bigger difference more expensive maintanance.
I believe in Linux situation is way different because in additional to huge players there is whole spectrum o medium and small one. In coreboot we have huge players, small players and community.
>
> > Consider: Even if a vendor doesn't intend to upstream, they still have to provide their sources to their customers (thanks to the GPL). Helping those customers upstream such custom development can go a long way to help bring separate lines of development back "home".
>
> That sounds like an incredibly unlikely scenario to justify extra churn, though. After all, all this is trying to do is trying to safe the integrator of those extra out-of-tree patches some effort. It only makes sense when the net effort saved will consistently be higher than the net effort spent. I really doubt "on the off chance that someone finds some third-party non-upstreamed tarball release somewhere that they then want to upstream" happens often enough to make it worth doing this for every SoC.
It is not so unlikely it is about convincing customer that keeping up with upstream is cheaper, then figuring out after some years they have way outdated firmware for their successful project and cost of rebase is way bigger then regular updates.
P.S. 10k+ employees corp vs small entity IMO largely define point of view and it would be great if whole discussion would go to some consensus making coreboot place for everyone not only for biggest or those who got contract with the biggest.
--
To view, visit https://review.coreboot.org/c/coreboot/+/52576
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic7c01a0f1008821ebbda43a426ba1e3d6410e861
Gerrit-Change-Number: 52576
Gerrit-PatchSet: 1
Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 05 May 2021 23:18:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Patrick Georgi <pgeorgi(a)google.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Martin Roth, chris wang, Ivy Jian, Felix Held.
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52950 )
Change subject: mb/google/mancomb: Implement tis_plat_irq_status
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/52950
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I141a504a827f37724fab0aaed7498fd543e471d6
Gerrit-Change-Number: 52950
Gerrit-PatchSet: 2
Gerrit-Owner: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Ivy Jian <ivy_jian(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: chris wang <Chris.Wang(a)amd.com>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: chris wang <Chris.Wang(a)amd.com>
Gerrit-Attention: Ivy Jian <ivy_jian(a)compal.corp-partner.google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 05 May 2021 23:15:16 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Martin Roth, Angel Pons.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/51133 )
Change subject: Add support to switch LSPCON modes
......................................................................
Patch Set 11:
(1 comment)
Commit Message:
https://review.coreboot.org/c/libgfxinit/+/51133/comment/3281fdaf_39fe60d6
PS11, Line 9: highly-cursed
although accurate, not sure this adds anything of value
--
To view, visit https://review.coreboot.org/c/libgfxinit/+/51133
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: libgfxinit
Gerrit-Branch: master
Gerrit-Change-Id: Idfa9bdff47a6591000cd5092d64a4cd4f8dbdc8d
Gerrit-Change-Number: 51133
Gerrit-PatchSet: 11
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Wed, 05 May 2021 23:00:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Martin Roth, chris wang, Ivy Jian, Felix Held.
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52949 )
Change subject: mb/google/mancomb: Update Kconfig with needed options
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/52949
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I39efcc8d1e0fb1e7ac0b0541a49db0ac0ee56481
Gerrit-Change-Number: 52949
Gerrit-PatchSet: 1
Gerrit-Owner: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Ivy Jian <ivy_jian(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Reviewer: chris wang <Chris.Wang(a)amd.com>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: chris wang <Chris.Wang(a)amd.com>
Gerrit-Attention: Ivy Jian <ivy_jian(a)compal.corp-partner.google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 05 May 2021 22:56:09 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Martin Roth, Felix Held.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52576 )
Change subject: Documentation: Add suggestion to document flag days
......................................................................
Patch Set 1:
(1 comment)
File Documentation/getting_started/gerrit_guidelines.md:
https://review.coreboot.org/c/coreboot/+/52576/comment/d2c8f4a5_0a19ec46
PS1, Line 332: requires a change in all mainboards using it) needs to be documented.
> Would "the code has shipped on a product" be a good break-off point?
Yes, for Chrome OS boards, but I'm not sure that's transferable to all coreboot developers. For the Facebook guys with their servers, for example, I don't know what a good cutoff point for them would be. I still think "once multiple independent parties say they are interested in it" for each SoC would be the best way to handle this to avoid a lot of unnecessary bookkeeping. (For a "single party", like Google and an SoC vendor collaborating on a new Chromebook, we usually just keep each other CCed in the actual patches as they go in, which is a much lower-touch way to keep each other in the loop.)
> any kind of rework of the code should be documented so that upstream changes can be brought into local stabilization branches (e.g. ChromeOS' firmware branches!)
Well, for Chrome OS firmware branches specifically, the kind of broad API changes we're talking about here usually *shouldn't* be backported. That's kinda what I was trying to say in my email as well, if you're trying to stabilize for a product you shouldn't *want* to keep pulling in every new hot-off-the-presses development from master. You should sit back and only take bug fixes relevant to your platform.
> Consider: Even if a vendor doesn't intend to upstream, they still have to provide their sources to their customers (thanks to the GPL). Helping those customers upstream such custom development can go a long way to help bring separate lines of development back "home".
That sounds like an incredibly unlikely scenario to justify extra churn, though. After all, all this is trying to do is trying to safe the integrator of those extra out-of-tree patches some effort. It only makes sense when the net effort saved will consistently be higher than the net effort spent. I really doubt "on the off chance that someone finds some third-party non-upstreamed tarball release somewhere that they then want to upstream" happens often enough to make it worth doing this for every SoC.
--
To view, visit https://review.coreboot.org/c/coreboot/+/52576
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic7c01a0f1008821ebbda43a426ba1e3d6410e861
Gerrit-Change-Number: 52576
Gerrit-PatchSet: 1
Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Martin Roth <martinroth(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 05 May 2021 22:50:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Patrick Georgi <pgeorgi(a)google.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/52862 )
Change subject: util/intelp2m: Set GO111MODULE environment parameter explicitly
......................................................................
util/intelp2m: Set GO111MODULE environment parameter explicitly
With go1.16 the default for GO111MODULE changed to on which break
building this tool.
Change-Id: I93a516ff76c8da4b7f37157d58ecd4c0b09c582c
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52862
Reviewed-by: Jonathan Zhang <jonzhang(a)fb.com>
Reviewed-by: David Hendricks <david.hendricks(a)gmail.com>
Reviewed-by: Maxim Polyakov <max.senia.poliak(a)gmail.com>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M util/intelp2m/Makefile
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
build bot (Jenkins): Verified
David Hendricks: Looks good to me, approved
Jonathan Zhang: Looks good to me, but someone else must approve
Maxim Polyakov: Looks good to me, approved
diff --git a/util/intelp2m/Makefile b/util/intelp2m/Makefile
index 1d9ba70..524ecd6 100644
--- a/util/intelp2m/Makefile
+++ b/util/intelp2m/Makefile
@@ -5,7 +5,7 @@
default:
go version
- go build -v -o $(PROJECT_NAME)
+ GO111MODULE=off go build -v -o $(PROJECT_NAME)
clean:
rm -Rf $(PROJECT_NAME) $(OUTPUT_DIR)
--
To view, visit https://review.coreboot.org/c/coreboot/+/52862
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I93a516ff76c8da4b7f37157d58ecd4c0b09c582c
Gerrit-Change-Number: 52862
Gerrit-PatchSet: 2
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: Maxim Polyakov <max.senia.poliak(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged