Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41036 )
Change subject: payloads/external/GRUB2: Makefile: fix check for changed files again
......................................................................
Patch Set 1:
please don't ask....
--
To view, visit https://review.coreboot.org/c/coreboot/+/41036
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I295c67ab8d7596bf54cc69d088ef1df906f58d5f
Gerrit-Change-Number: 41036
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Comment-Date: Mon, 04 May 2020 18:21:06 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40955 )
Change subject: payloads/external/GRUB2: prevent rebuild without actual changes
......................................................................
Uploaded patch set 4: Patch Set 3 was rebased.
--
To view, visit https://review.coreboot.org/c/coreboot/+/40955
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibf5a316d0c9761426ec2ee306d78cd56d4bf19b5
Gerrit-Change-Number: 40955
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Denis Carikli <GNUtoo(a)no-log.org>
Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Mon, 04 May 2020 18:16:28 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40955 )
Change subject: payloads/external/GRUB2: prevent rebuild without actual changes
......................................................................
Uploaded patch set 3.
--
To view, visit https://review.coreboot.org/c/coreboot/+/40955
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibf5a316d0c9761426ec2ee306d78cd56d4bf19b5
Gerrit-Change-Number: 40955
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Denis Carikli <GNUtoo(a)no-log.org>
Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Mon, 04 May 2020 17:57:01 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Hello build bot (Jenkins), Nico Huber, Patrick Georgi, Paul Menzel, Jonathan Neuschäfer, Angel Pons, Denis Carikli, Werner Zeh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40955
to look at the new patch set (#3).
Change subject: payloads/external/GRUB2: prevent rebuild without actual changes
......................................................................
payloads/external/GRUB2: prevent rebuild without actual changes
When the coreboot config changes, grub2 gets rebuilt, too. This is
caused by Make detecting a change in the dependency on the config file.
There are two cases where grub2 needs to be rebuilt (partly):
1. Changed list of included modules (CONFIG_GRUB2_EXTRA_MODULES)
In this case only the resulting binary has to be repackaged with
(or without) the modules changed. This does not require a full
rebuild and is intellegently handled by Make / the build system.
2. Changed grub revision or branch in coreboot config
In this case grub has to be recompiled from the new sources.
Fortunately all this is - again - magically handled by the build
system (automake and Make, to be precise) and there is no need to
influence that process from coreboot, like removing the build
directory for example.
This makes it possible to reduce build times after coreboot config
changes by adjusting the payload's Make targets and their dependencies
and letting the build system any required (partial or full) rebuilds.
Further, this commit removes the redundant dependeny on `build/config.h`
and drops the superfluous `CONFIG_DEP` make parameter.
Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de>
Change-Id: Ibf5a316d0c9761426ec2ee306d78cd56d4bf19b5
---
M payloads/external/GRUB2/Makefile
M payloads/external/Makefile.inc
2 files changed, 3 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/40955/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/40955
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibf5a316d0c9761426ec2ee306d78cd56d4bf19b5
Gerrit-Change-Number: 40955
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Denis Carikli <GNUtoo(a)no-log.org>
Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: newpatchset
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40955 )
Change subject: payloads/external/GRUB2: prevent rebuild without actual changes
......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40955/1/payloads/external/GRUB2/Ma…
File payloads/external/GRUB2/Makefile:
https://review.coreboot.org/c/coreboot/+/40955/1/payloads/external/GRUB2/Ma…
PS1, Line 40: grub2: $(CONFIG_DEP) checkout grub2/build/config.h
> Excuse my ignorance, but why is moving `CONFIG_DEP` fixes the issue? […]
Dropping config is just cosmetics, changing CONFIG_DEP is the actual change that makes the difference here:
`CONFIG_DEP` gets set in `payloads/external/Makefile.inc` to `CONFIG_DEP="$(abspath $(obj)/config.h)"`. Make/automake will rebuild a parent target (`grub2`) if one child (`grub2/build/config.h`) gets modified. That means in this case, that a dependency of `grub2` on `grub2/build/config.h` and a dependency of `grub2/build/config.h` on `build/config.h` will result in a rebuild of `grub2/build/config.h` (due to modified `build/config.h`) and finally in a rebuild of `grub2` (due to modified/rebuilt `grub2/build/config.h`).
And I just realized that the dependency of grub2 on `$(obj)/config.h` is already specified in `payloads/external/Makefile.inc` so there is absolutely no need to carry that `CONFIG_DEP` around. Thus, I will drop it, too.
--
To view, visit https://review.coreboot.org/c/coreboot/+/40955
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibf5a316d0c9761426ec2ee306d78cd56d4bf19b5
Gerrit-Change-Number: 40955
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Denis Carikli <GNUtoo(a)no-log.org>
Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Mon, 04 May 2020 17:53:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: comment
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40955 )
Change subject: payloads/external/GRUB2: prevent rebuild without actual changes
......................................................................
Uploaded patch set 2: Commit message was updated.
(2 comments)
https://review.coreboot.org/c/coreboot/+/40955/1//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/40955/1//COMMIT_MSG@12
PS1, Line 12: There are three
> Either you have forgotten the third one or you should say "two cases" here?
lol, indeed it should be "two"
https://review.coreboot.org/c/coreboot/+/40955/1//COMMIT_MSG@13
PS1, Line 13: 1. Changed list of included modules (CONFIG_GRUB2_EXTRA_MODULES)
> Please add a blank line above.
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/40955
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibf5a316d0c9761426ec2ee306d78cd56d4bf19b5
Gerrit-Change-Number: 40955
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Denis Carikli <GNUtoo(a)no-log.org>
Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Mon, 04 May 2020 17:43:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net>
Comment-In-Reply-To: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-MessageType: comment
Hello build bot (Jenkins), Nico Huber, Patrick Georgi, Paul Menzel, Jonathan Neuschäfer, Angel Pons, Denis Carikli, Werner Zeh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40955
to look at the new patch set (#2).
Change subject: payloads/external/GRUB2: prevent rebuild without actual changes
......................................................................
payloads/external/GRUB2: prevent rebuild without actual changes
When the coreboot config changes, grub2 gets rebuilt, too. This is
caused by Make detecting a change in the dependency on the config file.
There are two cases where grub2 needs to be rebuilt (partly):
1. Changed list of included modules (CONFIG_GRUB2_EXTRA_MODULES)
In this case only the resulting binary has to be repackaged with
(or without) the modules changed. This does not require a full
rebuild and is intellegently handled by Make / the build system.
2. Changed grub revision or branch in coreboot config
In this case grub has to be recompiled from the new sources.
Fortunately all this is - again - magically handled by the build
system (automake and Make, to be precise) and there is no need to
influence that process from coreboot, like removing the build
directory for example.
This makes it possible to reduce build times after coreboot config
changes by adjusting the payload's Make targets and their dependencies
and letting the build system any required (partial or full) rebuilds.
Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de>
Change-Id: Ibf5a316d0c9761426ec2ee306d78cd56d4bf19b5
---
M payloads/external/GRUB2/Makefile
1 file changed, 2 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/40955/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/40955
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibf5a316d0c9761426ec2ee306d78cd56d4bf19b5
Gerrit-Change-Number: 40955
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Niewöhner
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Denis Carikli <GNUtoo(a)no-log.org>
Gerrit-Reviewer: Jonathan Neuschäfer <j.neuschaefer(a)gmx.net>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: newpatchset
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41029 )
Change subject: soc/intel/jasperlake: Allow SataEnable to be filled from devicetree
......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/41029/4//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/41029/4//COMMIT_MSG@9
PS4, Line 9: it
it to 1
https://review.coreboot.org/c/coreboot/+/41029/4//COMMIT_MSG@10
PS4, Line 10: controller
Dot/period after *controller*.
https://review.coreboot.org/c/coreboot/+/41029/4//COMMIT_MSG@11
PS4, Line 11: it can be set as per each board's requirement.
1. Why can’t this be detected at run-time?
2. What is the the disadvantage of always enabling it?
--
To view, visit https://review.coreboot.org/c/coreboot/+/41029
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4f7e7508b8cd483508293ee3e7b760574d8f025f
Gerrit-Change-Number: 41029
Gerrit-PatchSet: 4
Gerrit-Owner: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Mon, 04 May 2020 17:06:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment