Change in coreboot[master]: Documentation/acpi: Add new document on adding ACPI devices to device...
Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35080 )
Change subject: Documentation/acpi: Add new document on adding ACPI devices to devicetree ......................................................................
Documentation/acpi: Add new document on adding ACPI devices to devicetree
Change-Id: I9636e65f7d2499b06b1d71e5f8d09c528b850027 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- A Documentation/acpi/devicetree.md 1 file changed, 191 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/35080/1
diff --git a/Documentation/acpi/devicetree.md b/Documentation/acpi/devicetree.md new file mode 100644 index 0000000..fdc36f1 --- /dev/null +++ b/Documentation/acpi/devicetree.md @@ -0,0 +1,191 @@ +# Adding new devices to a device tree + +## Introduction + +ACPI exposes a platform-independent interface for operating systems to +perform power management and other platform-level functions. Some +operating systems also use ACPI to enumerate devices that are not immediately +discoverable, such as those behind I2C or SPI busses (in contrast to +PCI). This document discusses the way that coreboot uses the concept +of a "device tree" to expose devices to the OS. + +## Devicetree and overridetree (if applicable) + +For mainboards that are organized around a "reference board" or "baseboard" +model (see _src/mainboard/google/octopus_ or _...hatch_ for examples), there is +typically a devicetree.cb file that all boards share, and any differences +for a specific board ("variant") are captured in the overridetree.cb file. Any settings +changed in the overridetree take precedence over those in the main devicetree. +Note that not all mainboards will have the devicetree/overridetree distinction, +and may only have a devicetree.cb file. Or you can always just write the AML code +yourself. + +## Device drivers + +Let's take a look at an example entry from _src/mainboard/google/hatch/variant/hatch/overridetree.cb_: + +``` +device pci 15.0 on + chip drivers/i2c/generic + register "hid" = ""ELAN0000"" + register "desc" = ""ELAN Touchpad"" + register "irq" = "ACPI_IRQ_WAKE_EDGE_LOW(GPP_A21_IRQ)" + register "wake" = "GPE0_DW0_21" + device i2c 15 on end + end +end # I2C #0 +``` + +When this entry is processed during ramstage, it will create a device in the +ACPI SSDT table (all devices in devicetrees end up in the SSDT table). The +ACPI generation routines in coreboot actually generate the raw bytecode that +represents the device's structure, but looking at AML code is easier to +understand; see below for what the disassembled bytecode looks like: + +``` +Scope (_SB.PCI0.I2C0) +{ + Device (D015) + { + Name (_HID, "ELAN0000") // _HID: Hardware ID + Name (_UID, Zero) // _UID: Unique ID + Name (_DDN, "ELAN Touchpad") // _DDN: DOS Device Name + Method (_STA, 0, NotSerialized) // _STA: Status + { + Return (0x0F) + } + Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings + { + I2cSerialBusV2 (0x0015, ControllerInitiated, 0x00061A80, + AddressingMode7Bit, "\_SB.PCI0.I2C0", + 0x00, ResourceConsumer, , Exclusive, ) + Interrupt (ResourceConsumer, Edge, ActiveLow, ExclusiveAndWake, ,, ) + { + 0x0000002D, + } + }) + Name (_S0W, 0x04) // _S0W: S0 Device Wake State + Name (_PRW, Package (0x02) // _PRW: Power Resources for Wake + { + 0x15, + 0x03 + }) + } +} +``` + +You can see it generates _HID, _UID, _DDN, _STA, _CRS, _S0W, and _PRW +names/methods in the Device's scope. Let's take a look at how the devicetree +language corresponds to the "generated AML". + +First, note this: + +``` + chip drivers/i2c/generic +``` + +This means that the device driver we're using has a corresponding structure, +located at _src/drivers/i2c/generic/chip.h_, named **struct drivers_i2c_generic_config** +and it contains many properties you can specify to be included in the ACPI +table. + +## Diving into the above example: + +### hid + +``` + register "hid" = ""ELAN0000"" +``` + +This corresponds to **const char *hid** in the struct. +In the ACPI AML, it translates to: + +``` + Name (_HID, "ELAN0000") // _HID: Hardware ID +``` + +under the Device. + +### desc + +``` + register "desc" = ""ELAN Touchpad"" +``` + +corresponds to **const char *desc** and in AML: + +``` + Name (_DDN, "ELAN Touchpad") // _DDN: DOS Device Name +``` + +### irq + +It also adds the interrupt, which comes from: + +``` + register "irq" = "ACPI_IRQ_WAKE_EDGE_LOW(GPP_A21_IRQ)" +``` + +(Note that the ACPI_IRQ_WAKE_EDGE_LOW macro informs the platform that the GPIO +will be routed through SCI (ACPI's System Control Interrupt) for use as a wake +source). + +### wake + +The last register is: + +``` + register "wake" = "GPE0_DW0_21" +``` + +which indicates that the method of waking the system using the touchpad will +be through a GPE, #21 associated with DW0 (which is setup in devicetree.cb +from this example). + +The last bit of the definition of that device includes: + +``` + device i2c 15 on end +``` + +which means it's an I2C device, with 7-bit address 0x15, and the device +is "on", meaning it will be exposed in the ACPI table. + +### Other auto-generated names + +### _S0W + _S0W indicates the deepest S0 sleep state that this device can wake itself +from, which in this case is 4, representing _D3cold_. + +### _PRW + _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 that supports waking the system (3), which +is S3. + +### _STA +The _STA method is generated automatically, and its values, 0xF, indicates the +following: + + Bit [0] – Set if the device is present. + Bit [1] – Set if the device is enabled and decoding its resources. + 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 +The _CRS (current resources) 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. + +``` +Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings +{ + I2cSerialBusV2 (0x0015, ControllerInitiated, 0x00061A80, + AddressingMode7Bit, "\_SB.PCI0.I2C0", + 0x00, ResourceConsumer, , Exclusive, ) +``` + +## Notes + + - **All fields that are left unspecified in the devicetree are initialized to zero.** + - **All devices in devicetrees end up in the SSDT table, and are generated in coreboot's ramstage**
Tim Wawrzynczak has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/35080 )
Change subject: Documentation/acpi: Add new document on adding ACPI devices to devicetree ......................................................................
Documentation/acpi: Add new document on adding ACPI devices to devicetree
Change-Id: I9636e65f7d2499b06b1d71e5f8d09c528b850027 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- A Documentation/acpi/devicetree.md M Documentation/index.md 2 files changed, 192 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/35080/2
Justin TerAvest has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35080 )
Change subject: Documentation/acpi: Add new document on adding ACPI devices to devicetree ......................................................................
Patch Set 2:
(5 comments)
Thanks for doing this!
https://review.coreboot.org/c/coreboot/+/35080/2/Documentation/acpi/devicetr... File Documentation/acpi/devicetree.md:
https://review.coreboot.org/c/coreboot/+/35080/2/Documentation/acpi/devicetr... PS2, Line 10: of a "device tree" to expose devices to the OS. May as well make it explicit here that devicetree is used in coreboot to generate ACPI tables.
https://review.coreboot.org/c/coreboot/+/35080/2/Documentation/acpi/devicetr... PS2, Line 20: and may only have a devicetree.cb file. Or you can always just write the AML code What does AML stand for?
https://review.coreboot.org/c/coreboot/+/35080/2/Documentation/acpi/devicetr... PS2, Line 70: 0x15, trailing whitespace here.
https://review.coreboot.org/c/coreboot/+/35080/2/Documentation/acpi/devicetr... PS2, Line 92: ## Diving into the above example: You can move this header up a bit to capture the chip entry as well.
https://review.coreboot.org/c/coreboot/+/35080/2/Documentation/acpi/devicetr... PS2, Line 156: ### _S0W I think markdown style requires a blank line after a header. What is _S0W short for? that might help here.
Hello Karthikeyan Ramasubramanian, Justin TerAvest, Duncan Laurie, build bot (Jenkins), Martin Roth, Furquan Shaikh, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35080
to look at the new patch set (#3).
Change subject: Documentation/acpi: Add new document on adding ACPI devices to devicetree ......................................................................
Documentation/acpi: Add new document on adding ACPI devices to devicetree
Change-Id: I9636e65f7d2499b06b1d71e5f8d09c528b850027 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- A Documentation/acpi/devicetree.md M Documentation/index.md 2 files changed, 196 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/35080/3
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35080 )
Change subject: Documentation/acpi: Add new document on adding ACPI devices to devicetree ......................................................................
Patch Set 2:
(5 comments)
https://review.coreboot.org/c/coreboot/+/35080/2/Documentation/acpi/devicetr... File Documentation/acpi/devicetree.md:
https://review.coreboot.org/c/coreboot/+/35080/2/Documentation/acpi/devicetr... PS2, Line 10: of a "device tree" to expose devices to the OS.
May as well make it explicit here that devicetree is used in coreboot to generate ACPI tables.
Done
https://review.coreboot.org/c/coreboot/+/35080/2/Documentation/acpi/devicetr... PS2, Line 20: and may only have a devicetree.cb file. Or you can always just write the AML code
What does AML stand for?
Done
https://review.coreboot.org/c/coreboot/+/35080/2/Documentation/acpi/devicetr... PS2, Line 70: 0x15,
trailing whitespace here.
Done
https://review.coreboot.org/c/coreboot/+/35080/2/Documentation/acpi/devicetr... PS2, Line 92: ## Diving into the above example:
You can move this header up a bit to capture the chip entry as well.
Done
https://review.coreboot.org/c/coreboot/+/35080/2/Documentation/acpi/devicetr... PS2, Line 156: ### _S0W
I think markdown style requires a blank line after a header. […]
Done
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35080 )
Change subject: Documentation/acpi: Add new document on adding ACPI devices to devicetree ......................................................................
Patch Set 3:
(9 comments)
https://review.coreboot.org/c/coreboot/+/35080/3/Documentation/acpi/devicetr... File Documentation/acpi/devicetree.md:
https://review.coreboot.org/c/coreboot/+/35080/3/Documentation/acpi/devicetr... PS3, Line 12: Devicetree and overridetree (if applicab This should really be a separate topic which you can link to eventually, but since nothing else exists yet...
https://review.coreboot.org/c/coreboot/+/35080/3/Documentation/acpi/devicetr... PS3, Line 20: AML : (ACPI Machine Language) code yourself I think you mean ASL (ACPI Source Language) here? that would be the other way to add a device (in this case into the DSDT).
Unless you mean having the mainboard use acpigen*() calls directly, which would technically be possible but I'm not sure I've seen it done that way.
https://review.coreboot.org/c/coreboot/+/35080/3/Documentation/acpi/devicetr... PS3, Line 42: AML I think you mean ASL here (the disassembled AML bytecode is the only thing that is really readable)
https://review.coreboot.org/c/coreboot/+/35080/3/Documentation/acpi/devicetr... PS3, Line 59: 0x00061A80 I would put this as 400000 so it is easier to see that it is just representing 400khz.
Either is valid to compile, but the disassembled code always uses hex.
https://review.coreboot.org/c/coreboot/+/35080/3/Documentation/acpi/devicetr... PS3, Line 70: 0x15, : 0x03 You cover it below but perhaps add a quick comment describing what these mean here as well.
https://review.coreboot.org/c/coreboot/+/35080/3/Documentation/acpi/devicetr... PS3, Line 87: chip drivers/i2c/generic This quickly gets out of hand if you follow everything that needs to be done, but it might be worth mentioning that the associated driver (i.e. CONFIG_DRIVERS_I2C_GENERIC here but typically found in the Kconfig file in the driver directory) needs to be enabled in the mainboard Kconfig as well or it is very confusing to debug why the code is not getting generated at boot.
https://review.coreboot.org/c/coreboot/+/35080/3/Documentation/acpi/devicetr... PS3, Line 127: GPP_A21_IRQ Also may be worth noting that this is an SOC-specific definition which may vary depending on the SOC.
https://review.coreboot.org/c/coreboot/+/35080/3/Documentation/acpi/devicetr... PS3, Line 127: ACPI_IRQ_WAKE_EDGE_LOW Perhaps point to arch/x86/include/arch/acpi_device.h for where to find these definitions.
https://review.coreboot.org/c/coreboot/+/35080/3/Documentation/acpi/devicetr... PS3, Line 149: device i2c 15 on end Might also mention that this ties into the PCI device definition to indicate which I2C bus this is present on.
i.e. "device pci 15.0 on" resolves to "Scope (_SB.PCI0.I2C0)" where this I2C slave address is located.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35080 )
Change subject: Documentation/acpi: Add new document on adding ACPI devices to devicetree ......................................................................
Patch Set 3:
(9 comments)
Thanks Duncan!
https://review.coreboot.org/c/coreboot/+/35080/3/Documentation/acpi/devicetr... File Documentation/acpi/devicetree.md:
https://review.coreboot.org/c/coreboot/+/35080/3/Documentation/acpi/devicetr... PS3, Line 12: Devicetree and overridetree (if applicab
This should really be a separate topic which you can link to eventually, but since nothing else exis […]
Fair point, I'll keep that on my todo-list.
https://review.coreboot.org/c/coreboot/+/35080/3/Documentation/acpi/devicetr... PS3, Line 20: AML : (ACPI Machine Language) code yourself
I think you mean ASL (ACPI Source Language) here? that would be the other way to add a device (in t […]
Sorry yes, I mean ASL not AML.
https://review.coreboot.org/c/coreboot/+/35080/3/Documentation/acpi/devicetr... PS3, Line 42: AML
I think you mean ASL here (the disassembled AML bytecode is the only thing that is really readable)
Ack
https://review.coreboot.org/c/coreboot/+/35080/3/Documentation/acpi/devicetr... PS3, Line 59: 0x00061A80
I would put this as 400000 so it is easier to see that it is just representing 400khz. […]
Weird, this is straight from iasl -d ssdt.dml. I'll update it anyhow.
https://review.coreboot.org/c/coreboot/+/35080/3/Documentation/acpi/devicetr... PS3, Line 70: 0x15, : 0x03
You cover it below but perhaps add a quick comment describing what these mean here as well.
Done
https://review.coreboot.org/c/coreboot/+/35080/3/Documentation/acpi/devicetr... PS3, Line 87: chip drivers/i2c/generic
This quickly gets out of hand if you follow everything that needs to be done, but it might be worth […]
Good idea, I'll add that up above when talking about device drivers.
https://review.coreboot.org/c/coreboot/+/35080/3/Documentation/acpi/devicetr... PS3, Line 127: ACPI_IRQ_WAKE_EDGE_LOW
Perhaps point to arch/x86/include/arch/acpi_device.h for where to find these definitions.
Done
https://review.coreboot.org/c/coreboot/+/35080/3/Documentation/acpi/devicetr... PS3, Line 127: GPP_A21_IRQ
Also may be worth noting that this is an SOC-specific definition which may vary depending on the SOC […]
Done
https://review.coreboot.org/c/coreboot/+/35080/3/Documentation/acpi/devicetr... PS3, Line 149: device i2c 15 on end
Might also mention that this ties into the PCI device definition to indicate which I2C bus this is p […]
Done
Hello Karthikeyan Ramasubramanian, Justin TerAvest, Duncan Laurie, build bot (Jenkins), Martin Roth, Furquan Shaikh, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35080
to look at the new patch set (#4).
Change subject: Documentation/acpi: Add new document on adding ACPI devices to devicetree ......................................................................
Documentation/acpi: Add new document on adding ACPI devices to devicetree
Change-Id: I9636e65f7d2499b06b1d71e5f8d09c528b850027 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- A Documentation/acpi/devicetree.md M Documentation/index.md 2 files changed, 211 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/35080/4
Hello Karthikeyan Ramasubramanian, Justin TerAvest, Duncan Laurie, build bot (Jenkins), Martin Roth, Furquan Shaikh, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35080
to look at the new patch set (#5).
Change subject: Documentation/acpi: Add new document on adding ACPI devices to devicetree ......................................................................
Documentation/acpi: Add new document on adding ACPI devices to devicetree
Change-Id: I9636e65f7d2499b06b1d71e5f8d09c528b850027 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- A Documentation/acpi/devicetree.md M Documentation/index.md 2 files changed, 211 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/35080/5
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35080 )
Change subject: Documentation/acpi: Add new document on adding ACPI devices to devicetree ......................................................................
Patch Set 5:
(10 comments)
Nice write-up. If you could wrap the lines, and mark up the paths, that’d be great.
https://review.coreboot.org/c/coreboot/+/35080/5/Documentation/acpi/devicetr... File Documentation/acpi/devicetree.md:
https://review.coreboot.org/c/coreboot/+/35080/5/Documentation/acpi/devicetr... PS5, Line 10: of a Fits on the line above?
https://review.coreboot.org/c/coreboot/+/35080/5/Documentation/acpi/devicetr... PS5, Line 19: Note that not all mainboards will have the devicetree/overridetree distinction, Note, not all …
https://review.coreboot.org/c/coreboot/+/35080/5/Documentation/acpi/devicetr... PS5, Line 25: Let's take a look at an example entry from _src/mainboard/google/hatch/variant/hatch/overridetree.cb_: Use `` to mark up code (monospace).
https://review.coreboot.org/c/coreboot/+/35080/5/Documentation/acpi/devicetr... PS5, Line 84: src/drivers/i2c/generic ``
https://review.coreboot.org/c/coreboot/+/35080/5/Documentation/acpi/devicetr... PS5, Line 86: mainboard's Kconfig file (e.g., src/mainboard/google/hatch/Kconfig) in order Mark up with ``.
https://review.coreboot.org/c/coreboot/+/35080/5/Documentation/acpi/devicetr... PS5, Line 117: Device device?
https://review.coreboot.org/c/coreboot/+/35080/5/Documentation/acpi/devicetr... PS5, Line 154: setup set up
https://review.coreboot.org/c/coreboot/+/35080/5/Documentation/acpi/devicetr... PS5, Line 176: _S0W indicates the deepest S0 sleep state that this device can wake itself Remove the space at the beginning?
https://review.coreboot.org/c/coreboot/+/35080/5/Documentation/acpi/devicetr... PS5, Line 176: that Remove.
https://review.coreboot.org/c/coreboot/+/35080/5/Documentation/acpi/devicetr... PS5, Line 182: that supports supporting
Hello Karthikeyan Ramasubramanian, Justin TerAvest, Duncan Laurie, build bot (Jenkins), Martin Roth, Furquan Shaikh, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35080
to look at the new patch set (#6).
Change subject: Documentation/acpi: Add new document on adding ACPI devices to devicetree ......................................................................
Documentation/acpi: Add new document on adding ACPI devices to devicetree
Change-Id: I9636e65f7d2499b06b1d71e5f8d09c528b850027 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- A Documentation/acpi/devicetree.md M Documentation/index.md 2 files changed, 216 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/35080/6
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35080 )
Change subject: Documentation/acpi: Add new document on adding ACPI devices to devicetree ......................................................................
Patch Set 6:
(10 comments)
https://review.coreboot.org/c/coreboot/+/35080/5/Documentation/acpi/devicetr... File Documentation/acpi/devicetree.md:
https://review.coreboot.org/c/coreboot/+/35080/5/Documentation/acpi/devicetr... PS5, Line 10: of a
Fits on the line above?
Done
https://review.coreboot.org/c/coreboot/+/35080/5/Documentation/acpi/devicetr... PS5, Line 19: Note that not all mainboards will have the devicetree/overridetree distinction,
Note, not all …
Done
https://review.coreboot.org/c/coreboot/+/35080/5/Documentation/acpi/devicetr... PS5, Line 25: Let's take a look at an example entry from _src/mainboard/google/hatch/variant/hatch/overridetree.cb_:
Use `` to mark up code (monospace).
Done
https://review.coreboot.org/c/coreboot/+/35080/5/Documentation/acpi/devicetr... PS5, Line 84: src/drivers/i2c/generic
``
Done
https://review.coreboot.org/c/coreboot/+/35080/5/Documentation/acpi/devicetr... PS5, Line 86: mainboard's Kconfig file (e.g., src/mainboard/google/hatch/Kconfig) in order
Mark up with ``.
Done
https://review.coreboot.org/c/coreboot/+/35080/5/Documentation/acpi/devicetr... PS5, Line 117: Device
device?
Done
https://review.coreboot.org/c/coreboot/+/35080/5/Documentation/acpi/devicetr... PS5, Line 154: setup
set up
Done
https://review.coreboot.org/c/coreboot/+/35080/5/Documentation/acpi/devicetr... PS5, Line 176: _S0W indicates the deepest S0 sleep state that this device can wake itself
Remove the space at the beginning?
Done
https://review.coreboot.org/c/coreboot/+/35080/5/Documentation/acpi/devicetr... PS5, Line 176: that
Remove.
Done
https://review.coreboot.org/c/coreboot/+/35080/5/Documentation/acpi/devicetr... PS5, Line 182: that supports
supporting
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35080 )
Change subject: Documentation/acpi: Add new document on adding ACPI devices to devicetree ......................................................................
Patch Set 6:
(5 comments)
https://review.coreboot.org/c/coreboot/+/35080/6/Documentation/acpi/devicetr... File Documentation/acpi/devicetree.md:
https://review.coreboot.org/c/coreboot/+/35080/6/Documentation/acpi/devicetr... PS6, Line 119: e. This property is used to match the device to its driver during enumeration in the OS.
https://review.coreboot.org/c/coreboot/+/35080/6/Documentation/acpi/devicetr... PS6, Line 141: informs the platform that the GPIO : will be routed through SCI (ACPI's System Control Interrupt) Also, that this is the Interrupt() resource that will be added to _CRS to provide information about the device interrupt to its driver. Here it is marked as active low and triggered on edge i.e. falling edge.
https://review.coreboot.org/c/coreboot/+/35080/6/Documentation/acpi/devicetr... PS6, Line 143: . This requires that the gpio GPP_A21 be configured in coreboot accordingly.
https://review.coreboot.org/c/coreboot/+/35080/6/Documentation/acpi/devicetr... PS6, Line 157: . It might be helpful to indicate #21 is actually GPP_X21 where GPP_X is mapped to DW0 in devicetree.cb
https://review.coreboot.org/c/coreboot/+/35080/6/Documentation/acpi/devicetr... PS6, Line 183: There are no : dependent power resources Are you planning on adding information about power resources later?
Hello Karthikeyan Ramasubramanian, Justin TerAvest, Duncan Laurie, build bot (Jenkins), Furquan Shaikh, Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35080
to look at the new patch set (#7).
Change subject: Documentation/acpi: Add new document on adding ACPI devices to devicetree ......................................................................
Documentation/acpi: Add new document on adding ACPI devices to devicetree
Change-Id: I9636e65f7d2499b06b1d71e5f8d09c528b850027 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- A Documentation/acpi/devicetree.md M Documentation/index.md 2 files changed, 235 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/35080/7
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35080 )
Change subject: Documentation/acpi: Add new document on adding ACPI devices to devicetree ......................................................................
Patch Set 7:
(5 comments)
https://review.coreboot.org/c/coreboot/+/35080/6/Documentation/acpi/devicetr... File Documentation/acpi/devicetree.md:
https://review.coreboot.org/c/coreboot/+/35080/6/Documentation/acpi/devicetr... PS6, Line 119: e.
This property is used to match the device to its driver during enumeration in the OS.
Done
https://review.coreboot.org/c/coreboot/+/35080/6/Documentation/acpi/devicetr... PS6, Line 141: informs the platform that the GPIO : will be routed through SCI (ACPI's System Control Interrupt)
Also, that this is the Interrupt() resource that will be added to _CRS to provide information about […]
Done
https://review.coreboot.org/c/coreboot/+/35080/6/Documentation/acpi/devicetr... PS6, Line 143: .
This requires that the gpio GPP_A21 be configured in coreboot accordingly.
Done
https://review.coreboot.org/c/coreboot/+/35080/6/Documentation/acpi/devicetr... PS6, Line 157: .
It might be helpful to indicate #21 is actually GPP_X21 where GPP_X is mapped to DW0 in devicetree. […]
Done
https://review.coreboot.org/c/coreboot/+/35080/6/Documentation/acpi/devicetr... PS6, Line 183: There are no : dependent power resources
Are you planning on adding information about power resources later?
Yes, that's probably worth a separate page / section. The details depend on whether or not the power resource refactoring gets merged or not ;)
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35080 )
Change subject: Documentation/acpi: Add new document on adding ACPI devices to devicetree ......................................................................
Patch Set 7: Code-Review+1
Thank you for the quick follow-ups.
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35080 )
Change subject: Documentation/acpi: Add new document on adding ACPI devices to devicetree ......................................................................
Patch Set 7: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35080 )
Change subject: Documentation/acpi: Add new document on adding ACPI devices to devicetree ......................................................................
Patch Set 7: Code-Review+2
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35080 )
Change subject: Documentation/acpi: Add new document on adding ACPI devices to devicetree ......................................................................
Patch Set 7: Code-Review+2
Aaron Durbin has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/35080 )
Change subject: Documentation/acpi: Add new document on adding ACPI devices to devicetree ......................................................................
Documentation/acpi: Add new document on adding ACPI devices to devicetree
Change-Id: I9636e65f7d2499b06b1d71e5f8d09c528b850027 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/35080 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Duncan Laurie dlaurie@chromium.org Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Karthik Ramasubramanian kramasub@google.com --- A Documentation/acpi/devicetree.md M Documentation/index.md 2 files changed, 235 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Duncan Laurie: Looks good to me, approved Paul Menzel: Looks good to me, but someone else must approve Furquan Shaikh: Looks good to me, approved Karthik Ramasubramanian: Looks good to me, approved
diff --git a/Documentation/acpi/devicetree.md b/Documentation/acpi/devicetree.md new file mode 100644 index 0000000..556c966 --- /dev/null +++ b/Documentation/acpi/devicetree.md @@ -0,0 +1,234 @@ +# Adding new devices to a device tree + +## Introduction + +ACPI exposes a platform-independent interface for operating systems to perform +power management and other platform-level functions. Some operating systems +also use ACPI to enumerate devices that are not immediately discoverable, such +as those behind I2C or SPI busses (in contrast to PCI). This document discusses +the way that coreboot uses the concept of a "device tree" to generate ACPI +tables for usage by the operating system. + +## Devicetree and overridetree (if applicable) + +For mainboards that are organized around a "reference board" or "baseboard" +model (see ``src/mainboard/google/octopus`` or ``hatch`` for examples), there is +typically a devicetree.cb file that all boards share, and any differences for a +specific board ("variant") are captured in the overridetree.cb file. Any +settings changed in the overridetree take precedence over those in the main +devicetree. Note, not all mainboards will have the devicetree/overridetree +distinction, and may only have a devicetree.cb file. Or you can always just +write the ASL (ACPI Source Language) code yourself. + +## Device drivers + +Let's take a look at an example entry from +``src/mainboard/google/hatch/variant/hatch/overridetree.cb``: + +``` +device pci 15.0 on + chip drivers/i2c/generic + register "hid" = ""ELAN0000"" + register "desc" = ""ELAN Touchpad"" + register "irq" = "ACPI_IRQ_WAKE_EDGE_LOW(GPP_A21_IRQ)" + register "wake" = "GPE0_DW0_21" + device i2c 15 on end + end +end # I2C #0 +``` + +When this entry is processed during ramstage, it will create a device in the +ACPI SSDT table (all devices in devicetrees end up in the SSDT table). The ACPI +generation routines in coreboot actually generate the raw bytecode that +represents the device's structure, but looking at ASL code is easier to +understand; see below for what the disassembled bytecode looks like: + +``` +Scope (_SB.PCI0.I2C0) +{ + Device (D015) + { + Name (_HID, "ELAN0000") // _HID: Hardware ID + Name (_UID, Zero) // _UID: Unique ID + Name (_DDN, "ELAN Touchpad") // _DDN: DOS Device Name + Method (_STA, 0, NotSerialized) // _STA: Status + { + Return (0x0F) + } + Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings + { + I2cSerialBusV2 (0x0015, ControllerInitiated, 400000, + AddressingMode7Bit, "\_SB.PCI0.I2C0", + 0x00, ResourceConsumer, , Exclusive, ) + Interrupt (ResourceConsumer, Edge, ActiveLow, ExclusiveAndWake, ,, ) + { + 0x0000002D, + } + }) + Name (_S0W, 0x04) // _S0W: S0 Device Wake State + Name (_PRW, Package (0x02) // _PRW: Power Resources for Wake + { + 0x15, // GPE #21 + 0x03 // Sleep state S3 + }) + } +} +``` + +You can see it generates _HID, _UID, _DDN, _STA, _CRS, _S0W, and _PRW +names/methods in the Device's scope. + +## Utilizing a device driver + +The device driver must be enabled for your build. There will be a CONFIG option +in the Kconfig file in the directory that the driver is in (e.g., +``src/drivers/i2c/generic`` contains a Kconfig file; the option here is named +CONFIG_DRIVERS_I2C_GENERIC). The config option will need to be added to your +mainboard's Kconfig file (e.g., ``src/mainboard/google/hatch/Kconfig``) in order +to be compiled into your build. + +## Diving into the above example: + +Let's take a look at how the devicetree language corresponds to the generated +ASL. + +First, note this: + +``` + chip drivers/i2c/generic +``` + +This means that the device driver we're using has a corresponding structure, +located at ``src/drivers/i2c/generic/chip.h``, named **struct +drivers_i2c_generic_config** and it contains many properties you can specify to +be included in the ACPI table. + +### hid + +``` + register "hid" = ""ELAN0000"" +``` + +This corresponds to **const char *hid** in the struct. In the ACPI ASL, it +translates to: + +``` + Name (_HID, "ELAN0000") // _HID: Hardware ID +``` + +under the device. **This property is used to match the device to its driver +during enumeration in the OS.** + +### desc + +``` + register "desc" = ""ELAN Touchpad"" +``` + +corresponds to **const char *desc** and in ASL: + +``` + Name (_DDN, "ELAN Touchpad") // _DDN: DOS Device Name +``` + +### irq + +It also adds the interrupt, + +``` + Interrupt (ResourceConsumer, Edge, ActiveLow, ExclusiveAndWake, ,, ) + { + 0x0000002D, + } +``` + +which comes from: + +``` + register "irq" = "ACPI_IRQ_WAKE_EDGE_LOW(GPP_A21_IRQ)" +``` + +The GPIO pin IRQ settings control the "Edge", "ActiveLow", and +"ExclusiveAndWake" settings seen above (edge means it is an edge-triggered +interrupt as opposed to level-triggered; active low means the interrupt is +triggered on a falling edge). + +Note that the ACPI_IRQ_WAKE_EDGE_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 +find the names in your SoC's header file. The ACPI_* macros are defined in +``src/arch/x86/include/arch/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 + +The last register is: + +``` + register "wake" = "GPE0_DW0_21" +``` + +which indicates that the method of waking the system using the touchpad will be +through a GPE, #21 associated with DW0, which is set up in devicetree.cb from +this example. The "21" indicates GPP_X21, where GPP_X is mapped onto DW0 +elsewhere in the devicetree. + +The last bit of the definition of that device includes: + +``` + device i2c 15 on end +``` + +which means it's an I2C device, with 7-bit address 0x15, and the device is "on", +meaning it will be exposed in the ACPI table. The PCI device that the +controller is located in determines which I2C bus the device is expected to be +found on. In this example, this is I2C bus 0. This also determines the ACPI +"Scope" that the device names and methods will live under, in this case +"_SB.PCI0.I2C0". + +## Other auto-generated names + +(see [ACPI specification +6.3](https://uefi.org/sites/default/files/resources/ACPI_6_3_final_Jan30.pdf) +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 4, representing _D3cold_. + +### _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 +following: + + Bit [0] – Set if the device is present. + Bit [1] – Set if the device is enabled and decoding its resources. + 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 +controller, and so specifies how to configure the controller for proper +operation with the touchpad. + +``` +Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings +{ + I2cSerialBusV2 (0x0015, ControllerInitiated, 400000, + AddressingMode7Bit, "\_SB.PCI0.I2C0", + 0x00, ResourceConsumer, , Exclusive, ) +``` + +## Notes + + - **All fields that are left unspecified in the devicetree are initialized to + zero.** + - **All devices in devicetrees end up in the SSDT table, and are generated in + coreboot's ramstage** diff --git a/Documentation/index.md b/Documentation/index.md index a2c2878..50141b5 100644 --- a/Documentation/index.md +++ b/Documentation/index.md @@ -172,6 +172,7 @@ * [Intel IFD Binary Extraction](Binary_Extraction.md) * [Dealing with Untrusted Input in SMM](technotes/2017-02-dealing-with-untrusted-input-in-smm.md) * [GPIO toggling in ACPI AML](acpi/gpio.md) +* [Adding devices to a device tree](acpi/devicetree.md) * [Native Graphics Initialization with libgfxinit](gfx/libgfxinit.md) * [Display panel-specific documentation](gfx/display-panel.md) * [Architecture-specific documentation](arch/index.md)
-
Aaron Durbin (Code Review) -
Duncan Laurie (Code Review)
-
Furquan Shaikh (Code Review)
-
Justin TerAvest (Code Review)
-
Karthik Ramasubramanian (Code Review)
-
Paul Menzel (Code Review)
-
Tim Wawrzynczak (Code Review)