Attention is currently required from: Angel Pons.
Nicholas Chin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58210 )
Change subject: util/autoport/bd82x6x.go: Fix includes
......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/58210/comment/936d47d0_d52d3250
PS1, Line 9: CB:49344
> Please refer to the commit hashes instead.
Done, hope I did this correctly.
--
To view, visit https://review.coreboot.org/c/coreboot/+/58210
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic1e97cfa7dce0e4d25f7a37c28d3635bdbf6c2a5
Gerrit-Change-Number: 58210
Gerrit-PatchSet: 3
Gerrit-Owner: Nicholas Chin
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Sun, 10 Oct 2021 19:17:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Nicholas Chin.
Hello build bot (Jenkins), Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/58210
to look at the new patch set (#3).
Change subject: util/autoport/bd82x6x.go: Fix includes
......................................................................
util/autoport/bd82x6x.go: Fix includes
Fix include of nvs.h to reflect
commit 661ad4666ca0e78195f6901fce7b44a7e56e6331;
and re-add <device/pci_ops.h>, removed in
commit 0aad0531dcbdd0628f9d16e27e34abb3f6e183aa,
as the generated early_init.c uses pci_write_config16().
Signed-off-by: Nicholas Chin <nic.c3.14(a)gmail.com>
Change-Id: Ic1e97cfa7dce0e4d25f7a37c28d3635bdbf6c2a5
---
M util/autoport/bd82x6x.go
1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/58210/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/58210
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic1e97cfa7dce0e4d25f7a37c28d3635bdbf6c2a5
Gerrit-Change-Number: 58210
Gerrit-PatchSet: 3
Gerrit-Owner: Nicholas Chin
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nicholas Chin
Gerrit-MessageType: newpatchset
Attention is currently required from: Nicholas Chin.
Hello build bot (Jenkins), Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/58210
to look at the new patch set (#2).
Change subject: util/autoport/bd82x6x.go: Fix includes
......................................................................
util/autoport/bd82x6x.go: Fix includes
Fix include of nvs.h to reflect
661ad4666ca0e78195f6901fce7b44a7e56e6331;
and re-add <device/pci_ops.h>, removed in
0aad0531dcbdd0628f9d16e27e34abb3f6e183aa,
as the generated early_init.c uses pci_write_config16().
Signed-off-by: Nicholas Chin <nic.c3.14(a)gmail.com>
Change-Id: Ic1e97cfa7dce0e4d25f7a37c28d3635bdbf6c2a5
---
M util/autoport/bd82x6x.go
1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/58210/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/58210
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic1e97cfa7dce0e4d25f7a37c28d3635bdbf6c2a5
Gerrit-Change-Number: 58210
Gerrit-PatchSet: 2
Gerrit-Owner: Nicholas Chin
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nicholas Chin
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Lance Zhao, Nico Huber, Furquan Shaikh, Tim Wawrzynczak, Paul Menzel, Angel Pons, Subrata Banik, Michael Niewöhner, Patrick Rudolph.
Hello Felix Singer, Lance Zhao, build bot (Jenkins), Nico Huber, Furquan Shaikh, Tim Wawrzynczak, Tim Wawrzynczak, Paul Menzel, Angel Pons, Subrata Banik, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/58018
to look at the new patch set (#16).
Change subject: soc/intel/pmc: add a note about legacy OSes/payloads to PM Timer Kconfig
......................................................................
soc/intel/pmc: add a note about legacy OSes/payloads to PM Timer Kconfig
Since ACPI 5.0A it is allowed to disable the ACPI Timer, when the
according FADT flag `ACPI_FADT_PLATFORM_CLOCK` is unset.
Starting with Skylake, most platforms (except Xeon-SP) support PM Timer
emulation, so even legacy OSes and payloads should work fine with the
hardware PM Timer disabled. However, when the `TMR_STS` functionality
is required, some legacy OSes might still not work (properly).
Add a note about this to the Kconfig help.
Change-Id: I53f1814113902124779ed85da030374439570688
Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de>
---
M src/soc/intel/common/block/pmc/Kconfig
1 file changed, 7 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/58018/16
--
To view, visit https://review.coreboot.org/c/coreboot/+/58018
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I53f1814113902124779ed85da030374439570688
Gerrit-Change-Number: 58018
Gerrit-PatchSet: 16
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: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Lance Zhao
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: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Lance Zhao
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Lance Zhao, Furquan Shaikh, Tim Wawrzynczak, Paul Menzel, Angel Pons, Subrata Banik, Michael Niewöhner, Patrick Rudolph.
Hello Felix Singer, Lance Zhao, build bot (Jenkins), Nico Huber, Furquan Shaikh, Tim Wawrzynczak, Tim Wawrzynczak, Paul Menzel, Angel Pons, Subrata Banik, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/58018
to look at the new patch set (#15).
Change subject: soc/intel/pmc: add a note about legacy OSes/payloads to PM Timer Kconfig
......................................................................
soc/intel/pmc: add a note about legacy OSes/payloads to PM Timer Kconfig
Since ACPI 5.0A it is allowed to disable the ACPI Timer, when the
according FADT flag `ACPI_FADT_PLATFORM_CLOCK` is unset.
Starting with Skylake, most platforms (except Xeon-SP) support PM Timer
emulation, so even legacy OSes and payloads should work fine with the
hardware PM Timer disabled. However, when the `TMR_STS` functionality
is required, some legacy OSes might still not work (properly).
Add a note about this to the Kconfig help.
Change-Id: I53f1814113902124779ed85da030374439570688
Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de>
---
M src/soc/intel/common/block/pmc/Kconfig
1 file changed, 7 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/18/58018/15
--
To view, visit https://review.coreboot.org/c/coreboot/+/58018
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I53f1814113902124779ed85da030374439570688
Gerrit-Change-Number: 58018
Gerrit-PatchSet: 15
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: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Lance Zhao
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: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Lance Zhao
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Lance Zhao, Furquan Shaikh, Tim Wawrzynczak, Paul Menzel, Angel Pons, Subrata Banik, Michael Niewöhner, Patrick Rudolph.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58018 )
Change subject: soc/intel/pmc: add a note about legacy OSes/payloads to PM Timer Kconfig
......................................................................
Patch Set 14: Code-Review+1
(1 comment)
File src/soc/intel/common/block/pmc/Kconfig:
https://review.coreboot.org/c/coreboot/+/58018/comment/0a70ecfc_e091c401
PS13, Line 80: or payloads with ACPI version < 5.0A might not work.
> currently only Xeon-SP (at least we don't know if it supports emulation)
Ack
The sentence needs some context, though. e.g. "might not work without PM timer.".
Or just Start the whole "Note" with "If disabled, ..."
--
To view, visit https://review.coreboot.org/c/coreboot/+/58018
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I53f1814113902124779ed85da030374439570688
Gerrit-Change-Number: 58018
Gerrit-PatchSet: 14
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: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Lance Zhao
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: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Lance Zhao
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Sun, 10 Oct 2021 17:01:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
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: Felix Singer, Lance Zhao, Jason Glenesk, Raul Rangel, Nico Huber, Furquan Shaikh, Marshall Dawson, Tim Wawrzynczak, Angel Pons, Patrick Rudolph, Felix Held.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58118 )
Change subject: acpigen,soc/amd/cezanne,intel/{common,skl}: rework CPPC table passing
......................................................................
Patch Set 9:
(1 comment)
Patchset:
PS8:
> To me this goes into the same direction as using a static ASL table: It creates
> obstacles for future code that might need more flexibility than the two tables
> we currently have. Trying to decide about such things is trying to predict the
> future :)
>
> Maybe there is a good compromise: Use `static const` tables where possible locally
> but keep the API (e.g. by using memcpy() to fill the passed struct). Or keep the
> struct and let the provider of the table decide if they point to a static struct
> or something more dynamic?
Yeah, doesn't sound bad
>
> I just noticed a hidden change here: Setting the version is moved to the place
> where the table is provided. Not sure what the original intention was wrt. this.
Very good question. Tbh I'm unsure the whole version thing.. When do we want/need to use a different version? Not sure if that's CPU or OS dependent.
> I think it makes sense to set it where the table is set. Also, the three-liner
> repeated in soc/intel/*, I guess that belongs in cpu/intel/common/?
You probably mean `cpu_get_cppc_table`? SKL will use common ACPI soon (CB:44138), so duplication will vanish.
>
> Out of the many changes, I think what I actually don't like is to remove the
> struct. Maybe we should keep the acpigen API as is but do all the other clean-up?
Ack
--
To view, visit https://review.coreboot.org/c/coreboot/+/58118
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I26c5e80c2a16a50ed73245c7c32d61b17e45c22a
Gerrit-Change-Number: 58118
Gerrit-PatchSet: 9
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: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Lance Zhao
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(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: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Lance Zhao
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
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-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Sun, 10 Oct 2021 16:59: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: Felix Singer, Raul Rangel, Furquan Shaikh, Paul Menzel, Angel Pons, Subrata Banik, Kyösti Mälkki, Patrick Rudolph, Jason Glenesk, Matt Delco, Nico Huber, Marshall Dawson, Tim Wawrzynczak, Felix Held.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57886 )
Change subject: acpigen,soc/amd,cpu/intel: rework static DWORD for CPPC table
......................................................................
Patch Set 11:
(1 comment)
File src/include/acpi/acpigen.h:
https://review.coreboot.org/c/coreboot/+/57886/comment/a3d8d5ca_8cd32671
PS10, Line 298: * NOTE: some fields permit DWORDs to be used. If you
: * provide a System Memory register with all zeros (which
: * represents unsupported) then this will be used as-is.
: * Otherwise, a System Memory register with a 32-bit
: * width will be converted into a DWORD field (the value
: * of which will be the value of 'addrl'. Any other use
: * of System Memory register is currently undefined.
: * (i.e., if you have an actual need for System Memory
: * then you'll need to adjust this kludge).
> This should be dropped or reworded.
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/57886
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib853261b5c0ea87ae2424fed188f2d1872be9a06
Gerrit-Change-Number: 57886
Gerrit-PatchSet: 11
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: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Lance Zhao
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Matt Delco <delco(a)chromium.org>
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: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Matt Delco <delco(a)chromium.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Sun, 10 Oct 2021 16:51:58 +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, Lance Zhao, Jason Glenesk, Raul Rangel, Furquan Shaikh, Marshall Dawson, Tim Wawrzynczak, Angel Pons, Michael Niewöhner, Patrick Rudolph, Felix Held.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58118 )
Change subject: acpigen,soc/amd/cezanne,intel/{common,skl}: rework CPPC table passing
......................................................................
Patch Set 9:
(1 comment)
Patchset:
PS8:
> Well, I did this as part of deobfuscating that whole code. […]
To me this goes into the same direction as using a static ASL table: It creates
obstacles for future code that might need more flexibility than the two tables
we currently have. Trying to decide about such things is trying to predict the
future :)
Maybe there is a good compromise: Use `static const` tables where possible locally
but keep the API (e.g. by using memcpy() to fill the passed struct). Or keep the
struct and let the provider of the table decide if they point to a static struct
or something more dynamic?
I just noticed a hidden change here: Setting the version is moved to the place
where the table is provided. Not sure what the original intention was wrt. this.
I think it makes sense to set it where the table is set. Also, the three-liner
repeated in soc/intel/*, I guess that belongs in cpu/intel/common/?
Out of the many changes, I think what I actually don't like is to remove the
struct. Maybe we should keep the acpigen API as is but do all the other clean-up?
--
To view, visit https://review.coreboot.org/c/coreboot/+/58118
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I26c5e80c2a16a50ed73245c7c32d61b17e45c22a
Gerrit-Change-Number: 58118
Gerrit-PatchSet: 9
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: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Lance Zhao
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(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: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Lance Zhao
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
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: Sun, 10 Oct 2021 16:48:05 +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