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