Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/79848?usp=email )
Change subject: sb/amd/pi/hudson/smhandler: use common APM_CNT_ACPI_* defines
......................................................................
sb/amd/pi/hudson/smhandler: use common APM_CNT_ACPI_* defines
The Hudson southbridge code for the AMD binaryPI SoCs had its own ACPI
enable and disable APMC command numbers that didn't match the common
defines in coreboot, so use the common define here to be consistent with
the command numbers in the corresponding FADT fields. Since the only SoC
that still would use this code doesn't select HAVE_SMI_HANDLER, this
won't fix any observable bug, but better fix this before anyone possibly
runs into this.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I5e596071e1b5269b616b7a93151648cb86ae77bc
---
M src/southbridge/amd/pi/hudson/hudson.h
M src/southbridge/amd/pi/hudson/smihandler.c
2 files changed, 2 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/79848/1
diff --git a/src/southbridge/amd/pi/hudson/hudson.h b/src/southbridge/amd/pi/hudson/hudson.h
index 510cdd0..96a5367 100644
--- a/src/southbridge/amd/pi/hudson/hudson.h
+++ b/src/southbridge/amd/pi/hudson/hudson.h
@@ -45,8 +45,6 @@
#define ACPI_SCI_IRQ 9
#define ACPI_SMI_CTL_PORT 0xb2
-#define ACPI_SMI_CMD_DISABLE 0xbe
-#define ACPI_SMI_CMD_ENABLE 0xef
#define REV_HUDSON_A11 0x11
#define REV_HUDSON_A12 0x12
diff --git a/src/southbridge/amd/pi/hudson/smihandler.c b/src/southbridge/amd/pi/hudson/smihandler.c
index 429d07a..966550f 100644
--- a/src/southbridge/amd/pi/hudson/smihandler.c
+++ b/src/southbridge/amd/pi/hudson/smihandler.c
@@ -28,12 +28,12 @@
const uint8_t cmd = inb(ACPI_SMI_CTL_PORT);
switch (cmd) {
- case ACPI_SMI_CMD_ENABLE:
+ case APM_CNT_ACPI_ENABLE:
reg32 = inl(ACPI_PM1_CNT_BLK);
reg32 |= (1 << 0); /* SCI_EN */
outl(reg32, ACPI_PM1_CNT_BLK);
break;
- case ACPI_SMI_CMD_DISABLE:
+ case APM_CNT_ACPI_DISABLE:
reg32 = inl(ACPI_PM1_CNT_BLK);
reg32 &= ~(1 << 0); /* clear SCI_EN */
outl(ACPI_PM1_CNT_BLK, reg32);
--
To view, visit https://review.coreboot.org/c/coreboot/+/79848?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I5e596071e1b5269b616b7a93151648cb86ae77bc
Gerrit-Change-Number: 79848
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newchange
Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/79846?usp=email )
Change subject: sb/amd/pi/hudson: fix gpio.h and smi.h include location
......................................................................
sb/amd/pi/hudson: fix gpio.h and smi.h include location
This fixes the following compile error when trying to build the APU2
board with HAVE_SMI_HANDLER selected and the NO_SMM select removed:
In file included from src/soc/amd/common/block/gpio/gpio.c:8:
src/include/gpio.h:6:10: fatal error: soc/gpio.h: No such file or directory
6 | #include <soc/gpio.h> /* IWYU pragma: export */
| ^~~~~~~~~~~~
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: Ie06044b12f5cbcc55a2706ec566afd2eb294c62b
---
M src/southbridge/amd/pi/hudson/Makefile.inc
R src/southbridge/amd/pi/hudson/include/soc/gpio.h
R src/southbridge/amd/pi/hudson/include/soc/smi.h
3 files changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/79846/1
diff --git a/src/southbridge/amd/pi/hudson/Makefile.inc b/src/southbridge/amd/pi/hudson/Makefile.inc
index a7be5a9..e7ec1b7 100644
--- a/src/southbridge/amd/pi/hudson/Makefile.inc
+++ b/src/southbridge/amd/pi/hudson/Makefile.inc
@@ -36,6 +36,8 @@
smm-y += smihandler.c
smm-y += smi_util.c
+CPPFLAGS_common += -I$(src)/southbridge/amd/pi/hudson/include
+
# ROMSIG At ROMBASE + 0x20000:
# +-----------+---------------+----------------+------------+
# |0x55AA55AA |EC ROM Address |GEC ROM Address |USB3 ROM |
diff --git a/src/southbridge/amd/pi/hudson/soc/gpio.h b/src/southbridge/amd/pi/hudson/include/soc/gpio.h
similarity index 100%
rename from src/southbridge/amd/pi/hudson/soc/gpio.h
rename to src/southbridge/amd/pi/hudson/include/soc/gpio.h
diff --git a/src/southbridge/amd/pi/hudson/soc/smi.h b/src/southbridge/amd/pi/hudson/include/soc/smi.h
similarity index 100%
rename from src/southbridge/amd/pi/hudson/soc/smi.h
rename to src/southbridge/amd/pi/hudson/include/soc/smi.h
--
To view, visit https://review.coreboot.org/c/coreboot/+/79846?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ie06044b12f5cbcc55a2706ec566afd2eb294c62b
Gerrit-Change-Number: 79846
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newchange
Attention is currently required from: David Ruth, Eric Lai, Nick Vaccaro, Subrata Banik.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79794?usp=email )
Change subject: mb/google/byra/var/*: Set WLAN device type back to pci
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> With this change incorporated, I only see _SB.PCI0.MCHC, and not _SB.PCI0.RP[..].MCHC devices. […]
the problem that prompted this change was that the RP04.MCHC device was being added to the LPI constraint list under the PEP device. I don't have access to a brya device currently to recheck, but if there's no bogus MCHC device getting added to the constraint list (which will cause Windows to BSOD, because there's an entry in the list which doesn't have a corresponding device in the dsdt/ssdt) then I'm good with the fix
--
To view, visit https://review.coreboot.org/c/coreboot/+/79794?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: If5dad9deb040c8cb0c507e11726f0ba44ccb2909
Gerrit-Change-Number: 79794
Gerrit-PatchSet: 1
Gerrit-Owner: David Ruth <druth(a)chromium.org>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: David Ruth <druth(a)chromium.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Comment-Date: Mon, 08 Jan 2024 15:43:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: David Ruth <druth(a)chromium.org>
Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Eric Lai, Matt DeVillier, Nick Vaccaro, Subrata Banik.
David Ruth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79794?usp=email )
Change subject: mb/google/byra/var/*: Set WLAN device type back to pci
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> if this is indeed necessary, then we need to figure out how to exclude these bogus device values fro […]
With this change incorporated, I only see _SB.PCI0.MCHC, and not _SB.PCI0.RP[..].MCHC devices. The device that is missing without this change (on the device I'm testing on, pujjo) is _SB.PCI0.RP01.WF00. I'm fairly new to this, but I ran the following commands:
```
mkdir acpi
cd acpi
acpidump -b
iasl *.dat
grep MCHC *dsl
```
And only came out with _SB.PCI0.MCHC in both dsdt and ssdt. Let me know if I missed something. Which table(s) are the _SB.PCI0.RP[..].MCHC devices present in when you're testing?
Effectively, changing WiFi devices from pci to generic results in them being detected as CNVi devices rather than PCIe. This breaks Linux's ability to associate the hardware with an ACPI device presentation and prevents adding additional functionality via new functions in SSDT.
--
To view, visit https://review.coreboot.org/c/coreboot/+/79794?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: If5dad9deb040c8cb0c507e11726f0ba44ccb2909
Gerrit-Change-Number: 79794
Gerrit-PatchSet: 1
Gerrit-Owner: David Ruth <druth(a)chromium.org>
Gerrit-Reviewer: Eric Lai <ericllai(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Eric Lai <ericllai(a)google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Comment-Date: Mon, 08 Jan 2024 15:30:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Dinesh Gehlot, Eran Mitrani, Kapil Porwal, Nick Vaccaro, Tarun.
Tim Crawford has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79631?usp=email )
Change subject: soc/intel: Add config to use S3 over S0ix
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/79631/comment/8835c17b_1c364512 :
PS2, Line 11: indicating to the OS that S3
: suspend is the preferred sleep method.
The description for the flag is:
> A one informs OSPM that the platform is able to
> achieve power savings in S0 similar to or better
> than those typically achieved in S3. In effect,
> when this bit is set it indicates that the system
> will achieve no power benefit by making a sleep
> transition to S3.
which I read as "tell the OS to prefer modern suspend over S3".
> If you don't set it the OS cannot suspend that way.
Windows probably has no means to control which is used, but Linux can override which is used by `/sys/power/mem_sleep`. But Linux also seems to always claim s2idle support, even when `s0ix_enable` is not set, so I'm not sure how it's determining that.
--
To view, visit https://review.coreboot.org/c/coreboot/+/79631?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ife98166338c5457fb2c7dad81a30e54f487495f6
Gerrit-Change-Number: 79631
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com>
Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tarun <tstuli(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Eran Mitrani <mitrani(a)google.com>
Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Tarun <tstuli(a)gmail.com>
Gerrit-Comment-Date: Mon, 08 Jan 2024 15:19:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: comment
Attention is currently required from: Jérémy Compostella, Matt DeVillier.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79834?usp=email )
Change subject: arch/x86/include/smm: improve documentation of call_smm
......................................................................
Patch Set 2:
(1 comment)
File src/arch/x86/include/smm_call.h:
https://review.coreboot.org/c/coreboot/+/79834/comment/042683ff_2402e770 :
PS1, Line 9: can then read
> how about 'will then read' or 'then reads'? 'can' makes it sound optional
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/79834?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I3566af191492ce00a3033335ff80e01c33e98e63
Gerrit-Change-Number: 79834
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Comment-Date: Mon, 08 Jan 2024 15:08:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-MessageType: comment
Attention is currently required from: Felix Held, Jérémy Compostella, Matt DeVillier.
Hello Jérémy Compostella, Matt DeVillier, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/79834?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review+1 by Matt DeVillier, Verified+1 by build bot (Jenkins)
Change subject: arch/x86/include/smm: improve documentation of call_smm
......................................................................
arch/x86/include/smm: improve documentation of call_smm
Since the inline assembly code doesn't make it exactly obvious how this
function to call the APMC SMI handler works in detail and especially how
it passes the sub-command and argument pointer to the SMI handler, add a
more detailed explanation as comment.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I3566af191492ce00a3033335ff80e01c33e98e63
---
M src/arch/x86/include/smm_call.h
1 file changed, 4 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/79834/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/79834?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I3566af191492ce00a3033335ff80e01c33e98e63
Gerrit-Change-Number: 79834
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79836?usp=email )
Change subject: soc/amd: use apm_get_apmc() in APMC SMI handler
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/79836?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Iac6b614d900e51d91a0c155116a5edc29775ea99
Gerrit-Change-Number: 79836
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Mon, 08 Jan 2024 14:59:59 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Held, Jérémy Compostella.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79828?usp=email )
Change subject: cpu/x86/smi_trigger: use call_smm
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/79828?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Iefbdb3d17932d6db6a17b5771436ede220c714fb
Gerrit-Change-Number: 79828
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Mon, 08 Jan 2024 14:59:35 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment