coreboot-gerrit May 2021

coreboot-gerrit@coreboot.org

1 participants
6041 discussions

Change in coreboot[master]: [RFC] device: Introduce new method for setting device states
by build bot (Jenkins) (Code Review) 01 May '21

01 May '21

Attention is currently required from: Felix Singer, 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] device: Introduce new method for setting device states ...................................................................... Patch Set 16: (1 comment) File src/device/devenmap.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-118143): https://review.coreboot.org/c/coreboot/+/52493/comment/764b9d5f_d469346e PS16, 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: 16 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: Sat, 01 May 2021 00:41:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [RFC] device: Introduce new method for setting device states
by Felix Singer (Code Review) 01 May '21

01 May '21

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 (#16). Change subject: [RFC] device: Introduce new method for setting device states ...................................................................... [RFC] device: 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/device/Makefile.inc A src/device/devenmap.c A src/include/device/devenmap.h 3 files changed, 41 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/52493/16 -- 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: 16 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

1 0

Change in coreboot[master]: soc/intel/skylake: Use set_dev_state_by_devicetree()
by Felix Singer (Code Review) 01 May '21

01 May '21

Attention is currently required from: Patrick Rudolph. Hello Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/52810 to look at the new patch set (#2). Change subject: soc/intel/skylake: Use set_dev_state_by_devicetree() ...................................................................... soc/intel/skylake: Use set_dev_state_by_devicetree() Change-Id: I75088098a4cceea2e860ca97c4853d2e5279c2f2 Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M src/soc/intel/skylake/chip.c M src/soc/intel/skylake/romstage/fsp_params.c 2 files changed, 36 insertions(+), 64 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/52810/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/52810 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I75088098a4cceea2e860ca97c4853d2e5279c2f2 Gerrit-Change-Number: 52810 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: [RFC] device: Introduce new method for setting device states
by build bot (Jenkins) (Code Review) 01 May '21

01 May '21

Attention is currently required from: Felix Singer, 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] device: Introduce new method for setting device states ...................................................................... Patch Set 15: (1 comment) File src/device/devenmap.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-118142): https://review.coreboot.org/c/coreboot/+/52493/comment/fa34b20b_05ac9184 PS15, 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: 15 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: Sat, 01 May 2021 00:36:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [RFC] device: Introduce new method for setting device states
by Felix Singer (Code Review) 01 May '21

01 May '21

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 (#15). Change subject: [RFC] device: Introduce new method for setting device states ...................................................................... [RFC] device: 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/device/Makefile.inc A src/device/devenmap.c A src/include/device/devenmap.h 3 files changed, 41 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/52493/15 -- 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: 15 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

1 0

Change in coreboot[master]: soc/intel/skylake: Use set_dev_state_by_devicetree()
by Michael Niewöhner (Code Review) 01 May '21

01 May '21

Attention is currently required from: Felix Singer, Patrick Rudolph. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52810 ) Change subject: soc/intel/skylake: Use set_dev_state_by_devicetree() ...................................................................... Patch Set 1: (3 comments) File src/soc/intel/skylake/chip.c: https://review.coreboot.org/c/coreboot/+/52810/comment/747a057a_fc394058 PS1, Line 461: (!xdci_can_enable()) : params->XdciEnable = 0; params->XdciEnable = xdci_can_enable(); File src/soc/intel/skylake/romstage/fsp_params.c: https://review.coreboot.org/c/coreboot/+/52810/comment/39d3d9f2_d4541927 PS1, Line 15: newline? https://review.coreboot.org/c/coreboot/+/52810/comment/eb1ed61a_e593ee04 PS1, Line 137: PCH_DEVFN_SMBUS }, : { &m_cfg->EnableTraceHub, P alignment -- To view, visit https://review.coreboot.org/c/coreboot/+/52810 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I75088098a4cceea2e860ca97c4853d2e5279c2f2 Gerrit-Change-Number: 52810 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Sat, 01 May 2021 00:29:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [RFC] device: Introduce new method for setting device states
by Michael Niewöhner (Code Review) 01 May '21

01 May '21

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] device: Introduce new method for setting device states ...................................................................... Patch Set 14: (3 comments) File src/include/device/devenmap.h: https://review.coreboot.org/c/coreboot/+/52493/comment/e578a174_122781f5 PS14, Line 12: const not sure if that's right. Nico? https://review.coreboot.org/c/coreboot/+/52493/comment/58c3c046_812fa5c6 PS14, Line 12: uint8_t probably const https://review.coreboot.org/c/coreboot/+/52493/comment/eb9a1b2c_9ea51070 PS14, Line 17: const drop here; keep in function definition; see comment here https://review.coreboot.org/c/coreboot/+/52493/comment/934c3a3b_2b638efb/ -- 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: 14 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: Sat, 01 May 2021 00:28:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [RFC] device: Introduce new method for setting device states
by build bot (Jenkins) (Code Review) 01 May '21

01 May '21

Attention is currently required from: Felix Singer, 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] device: Introduce new method for setting device states ...................................................................... Patch Set 14: (1 comment) File src/device/devenmap.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-118140): https://review.coreboot.org/c/coreboot/+/52493/comment/5f8970e8_65924d7e PS14, 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: 14 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: Sat, 01 May 2021 00:15:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/skylake: Use set_dev_state_by_devicetree()
by Felix Singer (Code Review) 01 May '21

01 May '21

Attention is currently required from: Patrick Rudolph. Felix Singer has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/52810 ) Change subject: soc/intel/skylake: Use set_dev_state_by_devicetree() ...................................................................... soc/intel/skylake: Use set_dev_state_by_devicetree() Change-Id: I75088098a4cceea2e860ca97c4853d2e5279c2f2 Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M src/soc/intel/skylake/chip.c M src/soc/intel/skylake/romstage/fsp_params.c 2 files changed, 36 insertions(+), 63 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/52810/1 diff --git a/src/soc/intel/skylake/chip.c b/src/soc/intel/skylake/chip.c index d4d8938..8830697 100644 --- a/src/soc/intel/skylake/chip.c +++ b/src/soc/intel/skylake/chip.c @@ -5,6 +5,7 @@ #include <fsp/api.h> #include <acpi/acpi.h> #include <console/console.h> +#include <device/devenmap.h> #include <device/device.h> #include <device/pci_ids.h> #include <fsp/util.h> @@ -234,6 +235,25 @@ mainboard_silicon_init_params(params); + const struct device_enable_map devmap[] = { + { &params->Device4Enable, SA_DEVFN_TS }, + { &params->GmmEnable, SA_DEVFN_GMM }, + { &params->SaImguEnable, SA_DEVFN_IMGU }, + { &tconfig->ChapDeviceEnable, SA_DEVFN_CHAP }, + { &params->PchCio2Enable, PCH_DEVFN_CIO }, + { &params->PchLanEnable, PCH_DEVFN_GBE }, + { &params->PchIshEnable, PCH_DEVFN_ISH }, + { &params->PchHdaEnable, PCH_DEVFN_HDA }, + { &params->ShowSpiController, PCH_DEVFN_SPI }, + { &params->ScsEmmcEnabled, PCH_DEVFN_EMMC }, + { &params->SataEnable, PCH_DEVFN_SATA }, + { &params->XdciEnable, PCH_DEVFN_USBOTG }, + { &params->ScsSdCardEnabled, PCH_DEVFN_SDCARD }, + { &params->PchThermalDeviceEnable, PCH_DEVFN_THERMAL }, + }; + + set_dev_state_by_devicetree(devmap, ARRAY_SIZE(devmap)); + struct soc_power_limits_config *soc_confg; config_t *confg = config_of_soc(); soc_confg = &confg->power_limits_config; @@ -283,8 +303,6 @@ } } - dev = pcidev_path_on_root(PCH_DEVFN_SATA); - params->SataEnable = dev && dev->enabled; if (params->SataEnable) { memcpy(params->SataPortsEnable, config->SataPortsEnable, sizeof(params->SataPortsEnable)); @@ -350,15 +368,6 @@ memcpy(params->SerialIoDevMode, config->SerialIoDevMode, sizeof(params->SerialIoDevMode)); - dev = pcidev_path_on_root(PCH_DEVFN_CIO); - params->PchCio2Enable = dev && dev->enabled; - - dev = pcidev_path_on_root(SA_DEVFN_IMGU); - params->SaImguEnable = dev && dev->enabled; - - dev = pcidev_path_on_root(SA_DEVFN_CHAP); - tconfig->ChapDeviceEnable = dev && dev->enabled; - dev = pcidev_path_on_root(PCH_DEVFN_CSE_3); params->Heci3Enabled = dev && dev->enabled; @@ -368,8 +377,6 @@ params->PchPmWoWlanDeepSxEnable = config->PchPmWoWlanDeepSxEnable; params->PchPmLanWakeFromDeepSx = config->WakeConfigPcieWakeFromDeepSx; - dev = pcidev_path_on_root(PCH_DEVFN_GBE); - params->PchLanEnable = dev && dev->enabled; if (params->PchLanEnable) { params->PchLanLtrEnable = config->EnableLanLtr; params->PchLanK1OffEnable = config->EnableLanK1Off; @@ -378,13 +385,8 @@ } params->SsicPortEnable = config->SsicPortEnable; - dev = pcidev_path_on_root(PCH_DEVFN_EMMC); - params->ScsEmmcEnabled = dev && dev->enabled; params->ScsEmmcHs400Enabled = config->ScsEmmcHs400Enabled; - dev = pcidev_path_on_root(PCH_DEVFN_SDCARD); - params->ScsSdCardEnabled = dev && dev->enabled; - if (!!params->ScsEmmcHs400Enabled && !!config->EmmcHs400DllNeed) { params->PchScsEmmcHs400DllDataValid = !!config->EmmcHs400DllNeed; @@ -394,22 +396,10 @@ config->ScsEmmcHs400TxDataDll; } - /* If ISH is enabled, enable ISH elements */ - dev = pcidev_path_on_root(PCH_DEVFN_ISH); - params->PchIshEnable = dev && dev->enabled; - - dev = pcidev_path_on_root(PCH_DEVFN_HDA); - params->PchHdaEnable = dev && dev->enabled; - params->PchHdaVcType = config->PchHdaVcType; params->PchHdaIoBufferOwnership = config->IoBufferOwnership; params->PchHdaDspEnable = config->DspEnable; - dev = pcidev_path_on_root(SA_DEVFN_TS); - params->Device4Enable = dev && dev->enabled; - dev = pcidev_path_on_root(PCH_DEVFN_THERMAL); - params->PchThermalDeviceEnable = dev && dev->enabled; - tconfig->PchLockDownGlobalSmi = config->LockDownConfigGlobalSmi; tconfig->PchLockDownRtcLock = config->LockDownConfigRtcLock; tconfig->PowerLimit4 = 0; @@ -466,23 +456,10 @@ for (i = 0; i < ARRAY_SIZE(config->domain_vr_config); i++) fill_vr_domain_config(params, i, &config->domain_vr_config[i]); - /* Show SPI controller if enabled in devicetree.cb */ - dev = pcidev_path_on_root(PCH_DEVFN_SPI); - params->ShowSpiController = dev && dev->enabled; - /* Enable xDCI controller if enabled in devicetree and allowed */ - dev = pcidev_path_on_root(PCH_DEVFN_USBOTG); - if (dev) { - if (!xdci_can_enable()) - dev->enabled = 0; - params->XdciEnable = dev->enabled; - } else { + /* Disable xDCI controller if not allowed */ + if (!xdci_can_enable()) params->XdciEnable = 0; - } - - /* Enable or disable Gaussian Mixture Model in devicetree */ - dev = pcidev_path_on_root(SA_DEVFN_GMM); - params->GmmEnable = dev && dev->enabled; /* * Send VR specific mailbox commands: diff --git a/src/soc/intel/skylake/romstage/fsp_params.c b/src/soc/intel/skylake/romstage/fsp_params.c index 294bf44..be61d87 100644 --- a/src/soc/intel/skylake/romstage/fsp_params.c +++ b/src/soc/intel/skylake/romstage/fsp_params.c @@ -3,6 +3,7 @@ #include <assert.h> #include <console/console.h> #include <cpu/x86/msr.h> +#include <device/devenmap.h> #include <fsp/util.h> #include <intelblocks/cpulib.h> #include <soc/iomap.h> @@ -11,6 +12,7 @@ #include <soc/romstage.h> #include <soc/soc_chip.h> + static void cpu_flex_override(FSP_M_CONFIG *m_cfg) { msr_t flex_ratio; @@ -27,7 +29,6 @@ FSP_M_TEST_CONFIG *m_t_cfg, const struct soc_intel_skylake_config *config) { - const struct device *dev; /* * To enable or disable the corresponding PEG root port you need to * add to the devicetree.cb: @@ -38,8 +39,6 @@ * If PEG port is not defined in the device tree, it will be disabled * in FSP */ - dev = pcidev_path_on_root(SA_DEVFN_PEG0); /* PEG 0:1:0 */ - m_cfg->Peg0Enable = dev && dev->enabled; if (m_cfg->Peg0Enable) { m_cfg->Peg0Enable = 2; m_cfg->Peg0MaxLinkWidth = config->Peg0MaxLinkWidth; @@ -52,8 +51,6 @@ m_t_cfg->Peg0Gen3EqPh3Method = 0; } - dev = pcidev_path_on_root(SA_DEVFN_PEG1); /* PEG 0:1:1 */ - m_cfg->Peg1Enable = dev && dev->enabled; if (m_cfg->Peg1Enable) { m_cfg->Peg1Enable = 2; m_cfg->Peg1MaxLinkWidth = config->Peg1MaxLinkWidth; @@ -63,8 +60,6 @@ m_t_cfg->Peg1Gen3EqPh3Method = 0; } - dev = pcidev_path_on_root(SA_DEVFN_PEG2); /* PEG 0:1:2 */ - m_cfg->Peg2Enable = dev && dev->enabled; if (m_cfg->Peg2Enable) { m_cfg->Peg2Enable = 2; m_cfg->Peg2MaxLinkWidth = config->Peg2MaxLinkWidth; @@ -109,10 +104,7 @@ static void soc_primary_gfx_config_params(FSP_M_CONFIG *m_cfg, const struct soc_intel_skylake_config *config) { - const struct device *dev; - - dev = pcidev_path_on_root(SA_DEVFN_IGD); - m_cfg->InternalGfx = !CONFIG(SOC_INTEL_DISABLE_IGD) && dev && dev->enabled; + m_cfg->InternalGfx = !CONFIG(SOC_INTEL_DISABLE_IGD) && m_cfg->InternalGfx; /* * If iGPU is enabled, set IGD stolen size to 64MB. The FBC @@ -132,12 +124,22 @@ void platform_fsp_memory_init_params_cb(FSPM_UPD *mupd, uint32_t version) { const struct soc_intel_skylake_config *config; - const struct device *dev; FSP_M_CONFIG *m_cfg = &mupd->FspmConfig; FSP_M_TEST_CONFIG *m_t_cfg = &mupd->FspmTestConfig; config = config_of_soc(); + const struct device_enable_map devmap[] = { + { &m_cfg->InternalGfx, SA_DEVFN_IGD }, + { &m_cfg->Peg0Enable, SA_DEVFN_PEG0 }, + { &m_cfg->Peg1Enable, SA_DEVFN_PEG1 }, + { &m_cfg->Peg2Enable, SA_DEVFN_PEG2 }, + { &m_cfg->SmbusEnable, PCH_DEVFN_SMBUS }, + { &m_cfg->EnableTraceHub, PCH_DEVFN_TRACEHUB }, + }; + + set_dev_state_by_devicetree(devmap, ARRAY_SIZE(devmap)); + soc_memory_init_params(m_cfg, config); soc_peg_init_params(m_cfg, m_t_cfg, config); @@ -154,15 +156,9 @@ /* DCI and TraceHub configs */ m_t_cfg->PchDciEn = config->PchDciEn; - dev = pcidev_path_on_root(PCH_DEVFN_TRACEHUB); - m_cfg->EnableTraceHub = dev && dev->enabled; m_cfg->TraceHubMemReg0Size = config->TraceHubMemReg0Size; m_cfg->TraceHubMemReg1Size = config->TraceHubMemReg1Size; - /* Enable SMBus controller */ - dev = pcidev_path_on_root(PCH_DEVFN_SMBUS); - m_cfg->SmbusEnable = dev && dev->enabled; - /* Set primary graphic device */ soc_primary_gfx_config_params(m_cfg, config); m_t_cfg->SkipExtGfxScan = config->SkipExtGfxScan; -- To view, visit https://review.coreboot.org/c/coreboot/+/52810 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I75088098a4cceea2e860ca97c4853d2e5279c2f2 Gerrit-Change-Number: 52810 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: [RFC] device: Introduce new method for setting device states
by Felix Singer (Code Review) 01 May '21

01 May '21

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 (#14). Change subject: [RFC] device: Introduce new method for setting device states ...................................................................... [RFC] device: 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/device/Makefile.inc A src/device/devenmap.c A src/include/device/devenmap.h 3 files changed, 41 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/52493/14 -- 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: 14 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

1 0

Jump to page:

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2021