Attention is currently required from: Nico Huber, Raul Rangel, Furquan Shaikh, Matt DeVillier, Angel Pons, Michael Niewöhner, Patrick Rudolph, Felix Held.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52493 )
Change subject: [RFC] soc/intel/skylake: Introduce new method for setting device states
......................................................................
Patch Set 12:
(4 comments)
File src/soc/intel/skylake/fspdevmap.h:
https://review.coreboot.org/c/coreboot/+/52493/comment/5ebd0b2f_9072bff7
PS9, Line 13: uint8_t
> `unsigned int` or `size_t` please. There is no reason to constrain the API.
Done
https://review.coreboot.org/c/coreboot/+/52493/comment/0a83f8ae_97921256
PS9, Line 13: const
> Got it, thanks for explaining :)
Done
File src/soc/intel/skylake/fspdevmap.c:
https://review.coreboot.org/c/coreboot/+/52493/comment/b7d72a59_4ab7977b
PS9, Line 9: uint8_t
> `unsigned int` or maybe `size_t` please.
Done
https://review.coreboot.org/c/coreboot/+/52493/comment/5eb59a17_a3f00051
PS9, Line 13: if (dev) {
> heh, I love it!
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/52493
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I70fde306c65a8881f565c5f923be20f380ea64d3
Gerrit-Change-Number: 52493
Gerrit-PatchSet: 12
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 30 Apr 2021 22:40:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Raul Rangel, Furquan Shaikh, Matt DeVillier, Angel Pons, Patrick Rudolph, Felix Held.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52493 )
Change subject: [RFC] soc/intel/skylake: Introduce new method for setting device states
......................................................................
Patch Set 12:
(1 comment)
File src/soc/intel/skylake/fspdevmap.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-118137):
https://review.coreboot.org/c/coreboot/+/52493/comment/6a3716f9_83b9b2d0
PS12, Line 14: if (dev) {
braces {} are not necessary for single statement blocks
--
To view, visit https://review.coreboot.org/c/coreboot/+/52493
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I70fde306c65a8881f565c5f923be20f380ea64d3
Gerrit-Change-Number: 52493
Gerrit-PatchSet: 12
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 30 Apr 2021 22:40:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nico Huber, Raul Rangel, Furquan Shaikh, Matt DeVillier, Angel Pons, Patrick Rudolph, Felix Held.
Hello build bot (Jenkins), Raul Rangel, Nico Huber, Furquan Shaikh, Matt DeVillier, Tim Wawrzynczak, Angel Pons, Michael Niewöhner, Patrick Rudolph, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/52493
to look at the new patch set (#12).
Change subject: [RFC] soc/intel/skylake: Introduce new method for setting device states
......................................................................
[RFC] soc/intel/skylake: Introduce new method for setting device states
We mostly use the same mechanism for setting the state of internal
devices, but we copy it over again and again. Thus, introduce a new
method trying to reduce redundant code.
Basically, this uses an array of structs containing the DEVFN number,
its corresponding FSP option and an indicator if the FSP option is
inverted. The struct maps the device to its related FSP option and
`set_dev_state_by_devicetree()` iterates over the array using the usual
mechanism.
Unresolved issues:
- Where should fspdevmap{.c,.h} be implemented?
Change-Id: I70fde306c65a8881f565c5f923be20f380ea64d3
Signed-off-by: Felix Singer <felixsinger(a)posteo.net>
---
M src/soc/intel/skylake/Makefile.inc
M src/soc/intel/skylake/chip.c
A src/soc/intel/skylake/fspdevmap.c
A src/soc/intel/skylake/fspdevmap.h
M src/soc/intel/skylake/romstage/Makefile.inc
M src/soc/intel/skylake/romstage/fsp_params.c
6 files changed, 69 insertions(+), 63 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/52493/12
--
To view, visit https://review.coreboot.org/c/coreboot/+/52493
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I70fde306c65a8881f565c5f923be20f380ea64d3
Gerrit-Change-Number: 52493
Gerrit-PatchSet: 12
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Nico Huber, Raul Rangel, Furquan Shaikh, Matt DeVillier, Angel Pons, Patrick Rudolph, Felix Held.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52493 )
Change subject: [RFC] soc/intel/skylake: Introduce new method for setting device states
......................................................................
Patch Set 11:
(1 comment)
File src/soc/intel/skylake/fspdevmap.c:
https://review.coreboot.org/c/coreboot/+/52493/comment/47fb37f8_9d0af7d2
PS9, Line 13: if (dev) {
> Most (if not all) of the original code always sets the (FSP) option. This […]
heh, I love it!
--
To view, visit https://review.coreboot.org/c/coreboot/+/52493
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I70fde306c65a8881f565c5f923be20f380ea64d3
Gerrit-Change-Number: 52493
Gerrit-PatchSet: 11
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 30 Apr 2021 22:28:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nico Huber, Paul Menzel, Tim Wawrzynczak, Angel Pons, Patrick Rudolph.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52809 )
Change subject: treewide: Kconfig: replace `def_bool n` by `bool`
......................................................................
Patch Set 1:
This change is ready for review.
--
To view, visit https://review.coreboot.org/c/coreboot/+/52809
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I27e89c66d18b7b3a5ac425eacea10df8c013814f
Gerrit-Change-Number: 52809
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Fri, 30 Apr 2021 22:24:49 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Raul Rangel, Furquan Shaikh, Matt DeVillier, Angel Pons, Michael Niewöhner, Patrick Rudolph, Felix Held.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52493 )
Change subject: [RFC] soc/intel/skylake: Introduce new method for setting device states
......................................................................
Patch Set 11:
(5 comments)
File src/soc/intel/skylake/chip.c:
https://review.coreboot.org/c/coreboot/+/52493/comment/d18fb70b_da785f39
PS10, Line 251: PCH_DEVFN_SDCARD },
: { ¶ms->PchThermalDeviceEnable,
> alignment
Done
https://review.coreboot.org/c/coreboot/+/52493/comment/5a4c9b1a_db5123d1
PS10, Line 252:
> tab
Done
File src/soc/intel/skylake/fspdevmap.h:
https://review.coreboot.org/c/coreboot/+/52493/comment/02d206d7_ece54dd4
PS9, Line 4: #include <fsp/api.h>
: #include <fsp/util.h>
> Better don't. It may create trouble if trying to share this with non-Intel.
Done
https://review.coreboot.org/c/coreboot/+/52493/comment/76c4e15c_e3d46216
PS9, Line 9: uint8_t
> `bool` from `stdbool.h`.
Done
https://review.coreboot.org/c/coreboot/+/52493/comment/6850a838_9669cbfa
PS9, Line 10: UINT8
> Better use `uint8_t` from `stdint.h`. It more portable and should […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/52493
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I70fde306c65a8881f565c5f923be20f380ea64d3
Gerrit-Change-Number: 52493
Gerrit-PatchSet: 11
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 30 Apr 2021 22:21:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Furquan Shaikh, Tim Wawrzynczak, Patrick Rudolph, Felix Held.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48934 )
Change subject: [RFC] device/pnp: do not warn for generic unassigned ressources
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
> I can't say that I know all SIOs. It seems possible that there are LDNs that if enabled need a specific register set.
How do you tell if a register *needs* to be set? I mean... the PCH has many registers, too, that *may* be programmed but don't *need* to be programmed. SIO registers are the same. They all have sensible defaults which *can* be changed but don't have to be.
--
To view, visit https://review.coreboot.org/c/coreboot/+/48934
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic23852d525a3f9c95118b113d419f815cac002d3
Gerrit-Change-Number: 48934
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 30 Apr 2021 21:07:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment