Raul Rangel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/67385 )
Change subject: Documentation: Add wake source info to device tree documentation ......................................................................
Documentation: Add wake source info to device tree documentation
The device tree documentation was promoting using a GPIO wake event and a GPE wake event. We should only ever have one. This wasn't actually causing a problem because the wake bit was set on the `irq` property, but the IO-APIC can't actually wake the system, so it was a no-op.
This change fixes up the markdown so it's formatted correctly, and also adds a section explaining what the different wake configurations are.
BUG=b:243700486 TEST=mdformat
Signed-off-by: Raul E Rangel rrangel@chromium.org Change-Id: Ifcdbd5371408784bf9b81c1ade90263de8c60e0f --- M Documentation/getting_started/devicetree.md 1 file changed, 99 insertions(+), 27 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/67385/1
diff --git a/Documentation/getting_started/devicetree.md b/Documentation/getting_started/devicetree.md index 41f5901..09f161b 100644 --- a/Documentation/getting_started/devicetree.md +++ b/Documentation/getting_started/devicetree.md @@ -86,7 +86,7 @@ chip drivers/i2c/generic register "hid" = ""ELAN0000"" register "desc" = ""ELAN Touchpad"" - register "irq" = "ACPI_IRQ_WAKE_LEVEL_LOW(GPP_A21_IRQ)" + register "irq" = "ACPI_IRQ_LEVEL_LOW(GPP_A21_IRQ)" register "wake" = "GPE0_DW0_21" device i2c 15 on end end @@ -116,7 +116,7 @@ I2cSerialBusV2 (0x0015, ControllerInitiated, 400000, AddressingMode7Bit, "\_SB.PCI0.I2C0", 0x00, ResourceConsumer, , Exclusive, ) - Interrupt (ResourceConsumer, Level, ActiveLow, ExclusiveAndWake, ,, ) + Interrupt (ResourceConsumer, Level, ActiveLow, Exclusive, ,, ) { 0x0000002D, } @@ -131,7 +131,7 @@ } ```
-You can see it generates _HID, _UID, _DDN, _STA, _CRS, _S0W, and _PRW +You can see it generates _HID, _UID, _DDN, _STA, _CRS, _S0W, and _PRW names/methods in the Device's scope.
## Utilizing a device driver @@ -165,7 +165,7 @@ register "hid" = ""ELAN0000"" ```
-This corresponds to **const char *hid** in the struct. In the ACPI ASL, it +This corresponds to **const char *hid** in the struct. In the ACPI ASL, it translates to:
``` @@ -181,18 +181,18 @@ register "desc" = ""ELAN Touchpad"" ```
-corresponds to **const char *desc** and in ASL: +corresponds to **const char *desc** and in ASL:
``` Name (_DDN, "ELAN Touchpad") // _DDN: DOS Device Name ```
-### irq +#### irq
It also adds the interrupt,
``` - Interrupt (ResourceConsumer, Level, ActiveLow, ExclusiveAndWake, ,, ) + Interrupt (ResourceConsumer, Level, ActiveLow, Exclusive, ,, ) { 0x0000002D, } @@ -201,24 +201,20 @@ which comes from:
``` - register "irq" = "ACPI_IRQ_WAKE_LEVEL_LOW(GPP_A21_IRQ)" + register "irq" = "ACPI_IRQ_LEVEL_LOW(GPP_A21_IRQ)" ```
-The GPIO pin IRQ settings control the "Level", "ActiveLow", and -"ExclusiveAndWake" settings seen above (level means it is a level-triggered -interrupt as opposed to edge-triggered; active low means the interrupt is -triggered when the signal is low). +The GPIO pin IRQ settings control the "Level", and "ActiveLow" settings seen +above (level means it is a level-triggered interrupt as opposed to +edge-triggered; active low means the interrupt is triggered when the signal is +low).
-Note that the ACPI_IRQ_WAKE_LEVEL_LOW macro informs the platform that the GPIO -will be routed through SCI (ACPI's System Control Interrupt) for use as a wake -source. Also note that the IRQ names are SoC-specific, and you will need to + +Also note that the IRQ names are SoC-specific, and you will need to find the names in your SoC's header file. The ACPI_* macros are defined in ``src/arch/x86/include/acpi/acpi_device.h``.
-Using a GPIO as an IRQ requires that it is configured in coreboot correctly. -This is often done in a mainboard-specific file named ``gpio.c``. - -### wake +#### wake
The last register is:
@@ -231,6 +227,8 @@ this example. The "21" indicates GPP_X21, where GPP_X is mapped onto DW0 elsewhere in the devicetree.
+### device + The last bit of the definition of that device includes:
``` @@ -244,6 +242,57 @@ "Scope" that the device names and methods will live under, in this case "_SB.PCI0.I2C0".
+## Wake sources + +The ACPI spec defines two methods to describe how a device can wake the system. +Only one of these method should be used, otherwise duplicate wake events will be +generated. + +### Using GPEs as a wake source + +The `wake` property specified above is used to tell the ACPI subsystem that the +device can use a GPE to wake the system. The OS can control whether to enable +or disable the wake source by unmasking/masking off the GPE. + +The GPIO -> GPE mapping must be configured in firmware. On AMD platforms this is +generally done by a mainboard specific `gpio.c` file that defines the GPIO +using `PAD_SCI`. The GPIO -> GPE mapping is returned by the +`soc_get_gpio_event_table` method that is defined in the SoC specific `gpio.c` +file. + +The linux kernel has great support for this method. + +Windows on the other hand will +[BSOD](https://github.com/MicrosoftDocs/windows-driver-docs/blob/staging/windows-dr...) +complaining about an invalid ACPI configuration. +> 0x1000D - A device used both GPE and GPIO interrupts, which is not supported. + +## Using GPIO interrupts as a wake source + +The `ACPI_IRQ_WAKE_{EDGE,LEVEL}_{LOW,HIGH}` macros can be used when setting the +`irq` or `gpio_irq` properties. This ends up setting `ExclusiveAndWake` or +`SharedAndWake` on the `Interrupt` or `GpioInt` ACPI resource. + +This method has a few caveats: +* On Intel and AMD platforms the IO-APIC can't wake the system. This means you + can't use the `ACPI_IRQ_WAKE_*` macros with the `irq` property. Instead you + need to use the `gpio_irq` property. +* The OS needs to know how to enable the `wake` bit on the GPIO. For linux this + means the platform specific GPIO controller driver must implement the + `irq_set_wake` callback. For AMD systems this wasn't implemented until + linux v5.15. If the controller doesn't define this callback, it's + possible for the firmware to manually set the `wake` bit on the GPIO. This is + often done in a mainboard-specific file named `gpio.c`. This is not + recommended because then it's not possible for the OS to disable the wake + source. +* As of linux v6.0, the ACPI subsystem doesn't take the GPIO `wake` bit into + account when deciding on which power state to put the device in before + suspending the system. This means that if you define a power resource for a + device via `has_power_resource`, `enable_gpio`, etc, then the linux kernel + will place the device into D3Cold. + +This is the method preferred by Windows since it doesn't result in a BSOD. + ## Other auto-generated names
(see [ACPI specification @@ -251,17 +300,19 @@ for more details on ACPI methods)
### _S0W (S0 Device Wake State) -_S0W indicates the deepest S0 sleep state this device can wake itself from, -which in this case is ACPI_DEVICE_SLEEP_D3_HOT, representing _D3hot_. +_S0W indicates the deepest S0 sleep state this device can wake itself from, +which in this case is `ACPI_DEVICE_SLEEP_D3_HOT`, representing _D3hot_. +D3Hot means the `PR3` power resources are still on and the device is still +responsive on the bus. For i2c devices this is generally the same state as `D0`.
-### _PRW (Power Resources for Wake) -_PRW indicates the power resources and events required for wake. There are no +### _PRW (Power Resources for Wake) +_PRW indicates the power resources and events required for wake. There are no dependent power resources, but the GPE (GPE0_DW0_21) is mentioned here (0x15), as well as the deepest sleep state supporting waking the system (3), which is S3.
-### _STA (Status) -The _STA method is generated automatically, and its values, 0xF, indicates the +### _STA (Status) +The _STA method is generated automatically, and its values, 0xF, indicates the following:
Bit [0] – Set if the device is present. @@ -269,8 +320,8 @@ Bit [2] – Set if the device should be shown in the UI. Bit [3] – Set if the device is functioning properly (cleared if device failed its diagnostics).
-### _CRS (Current resource settings) -The _CRS method is generated automatically, as the driver knows it is an I2C +### _CRS (Current resource settings) +The _CRS method is generated automatically, as the driver knows it is an I2C controller, and so specifies how to configure the controller for proper operation with the touchpad.