Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33033 )
Change subject: superio/common: Add ssdtgen for generic SuperIOs ......................................................................
Patch Set 25:
(7 comments)
https://review.coreboot.org/c/coreboot/+/33033/25/src/superio/common/generic... File src/superio/common/generic.c:
https://review.coreboot.org/c/coreboot/+/33033/25/src/superio/common/generic... PS25, Line 100: FIELDLIST_NAMESTR("IOAL", 8),
IOH1, IOL1 missing?
It's usually called IOA*
https://review.coreboot.org/c/coreboot/+/33033/25/src/superio/common/generic... PS25, Line 105: FIELDLIST_NAMESTR("INTT", 4),
second irq missing
Ack
https://review.coreboot.org/c/coreboot/+/33033/25/src/superio/common/ssdt.c File src/superio/common/ssdt.c:
https://review.coreboot.org/c/coreboot/+/33033/25/src/superio/common/ssdt.c@... PS25, Line 101: while (size > 0) {
hmm, for a 256 byte big resource this will generate a 255 and a 1 byte acpi io resource. […]
it is in bytes
https://review.coreboot.org/c/coreboot/+/33033/25/src/superio/common/ssdt.c@... PS25, Line 103: 0
AddressAlignment == 0 means fixed, right?
I guess 1 would be correct
https://review.coreboot.org/c/coreboot/+/33033/25/src/superio/common/ssdt.c@... PS25, Line 110: >
shouldn't this be a >= here?
Ack
https://review.coreboot.org/c/coreboot/+/33033/25/src/superio/common/ssdt.c@... PS25, Line 147: snprintf(name, sizeof(name), "VLD%X", vldn);
for the vldn the base ldn also needs to be known; only the enable bit position (the virtual part of […]
it makes no assumptions. Will add LDN, too.
https://review.coreboot.org/c/coreboot/+/33033/25/src/superio/common/ssdt.c@... PS25, Line 226: ldn_gen_resources_use(dev);
this doesn't need a name, header and footer?
it just outputs the integers "IOxB"/"IOxS" for every resource in the LDN scope. It doesn't generate an acpi resource object.