Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41553 )
Change subject: device/pci: Don't enabled unassigned resources ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41553/1/src/device/pci_device.c File src/device/pci_device.c:
https://review.coreboot.org/c/coreboot/+/41553/1/src/device/pci_device.c@575 PS1, Line 575: for (res = dev->resource_list; res; res = res->next) : pci_set_resource(dev, res, &command_mask); : /* If there are unassigned resources, we might : have to disable others of the same type. */ : dev->command &= command_mask; :
I am thinking if we should just split bridge and non-bridge resource setting completely.
I thought about that too, but the bridge and non-bridge resources share the command register bits. That makes it more delicate so I went for an less invasive change. Basically we have 4 cases:
* No resources => don't set command bit * Any non-bridge resource is invalid => don't set command bit * All resources are valid => set command bit * All non-bridge resources are valid, but a bridge one is not => we can disable bridge resources by programming limit < base, and set command bit
So we need to keep track of all the good and all the bad cases and will have to decide in the end. With this change, `dev->command` would gather the good and `command_mask` the bad cases.
I'll have a look into the split. If we also factor set_resource_command_bits() out, we could get away without redundancy.