[coreboot-gerrit] [M] Change in coreboot[master]: UNTESTED acpi: Add ECAM region below PNP0C02 device in SSDT

[Nico Huber (Code Review)]

Attention is currently required from: Angel Pons, Arthur Heymans, Jeff Daly, Lance Zhao, Matt DeVillier, Sean Rhodes, Tim Wawrzynczak, Vanessa Eusebio.

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75729?usp=email )

Change subject: UNTESTED acpi: Add ECAM region below PNP0C02 device in SSDT
......................................................................

Patch Set 3:

(2 comments)

File src/acpi/acpi.c:

https://review.coreboot.org/c/coreboot/+/75729/comment/6261b0a4_b381e99f :
PS2, Line 587: 	struct device *dev = dev_find_path(NULL, DEVICE_PATH_DOMAIN);
             : 	assert(dev != NULL);
> > We could also export domain devices to `build/static_devices.h`. […]

Right. I don't remember exactly what I imagined. Probably that we always
have a `domain 0` and only need ECAM for this one?

Anyway, a different thought: This is probably not the right place to decide
what device we add the resource to. Technically we already have the
`.acpi_fill_ssdt` pointer for this. We could set it to a default implementation
depending on CONFIG_ECAM_MMCONF_SUPPORT. And if a platform has its own
fill_ssdt(), it could still call the default one. WDYT?

https://review.coreboot.org/c/coreboot/+/75729/comment/31294bc5_0750b6fd :
PS2, Line 594: 1
> > Shouldn't this be unique (I'm not sure about the scope)? i.e. not the same […]

It should be unique among "PNP0C02" devices. I don't think we need _UID here.
It's probably better not to have one than a wrong one.