Attention is currently required from: Angel Pons, Arthur Heymans, Felix Held, Krystian Hebel, Kyösti Mälkki, Nico Huber, Patrick Rudolph.
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77338?usp=email )
Change subject: device/pciexp_device.c: Fix setting Max Payload Size ......................................................................
Patch Set 14:
(2 comments)
File src/device/pciexp_device.c:
https://review.coreboot.org/c/coreboot/+/77338/comment/fab04332_b926cf7f : PS11, Line 598: awlays Just to confirm we are on the same page. Patchset 14 currently does the following (in chronological order of execution):
1. Sets the bridge's MPS to its max cap in pciexp_scan_bus. 2. Sets device MPS to its max cap in pciexp_tune_dev. 3. In pciexp_tune_dev, traverses the topology up to eventually limit the currently configured MPS to the lower MPS of the device and its parent. 4. At the end of pciexp_scan_bus the MPS of root port is programmed to all devices beneath it.
I have removed the state tracking as per your idea:
On the way down, set every device to its own maximum. On the way up, propagate the minimum from the endpoints up to the root port
This would work without additional state tracking.
Is there any way to make it simpler? According to your comment:
With the max_payload_set state, we can also do everything on the way up.
Then we would have to look at the already set value and the cap maximum (like the current implementation does).
it looks like there is such a way. Although, I can't see how "do everything the way up" can be done, so any further suggestions are appreciated.
File src/device/pciexp_device.c:
https://review.coreboot.org/c/coreboot/+/77338/comment/4ab2e770_9415d996 : PS13, Line 755: (child->upstream->secondary > max_bus))
`child->upstream` is the same as `bus` by definition. So this check […]
Removed