Attention is currently required from: Angel Pons, Arthur Heymans, Felix Held, Krystian Hebel, Kyösti Mälkki, Michał Żygowski, Patrick Rudolph.
Nico Huber 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 13:
(2 comments)
File src/device/pciexp_device.c:
https://review.coreboot.org/c/coreboot/+/77338/comment/2929f70c_4a114110 : PS11, Line 598: awlays
Ad.1. This happens right now in patchset 11 AFAIU.
It did more than that. And now the code around 1. has too many redundant paths. Not sure if this help: pciexp_scan_bus() is already recursive. Every- thing that is called before the recursion happens on the way down, everything after happens on the way up. So we don't need additional loops that walk up, we do it already.
We should first decide on a specific strategy, then try to implement that. I already mentioned
- 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.
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).
File src/device/pciexp_device.c:
https://review.coreboot.org/c/coreboot/+/77338/comment/565ab89e_8ba54ecb : PS13, Line 755: (child->upstream->secondary > max_bus)) `child->upstream` is the same as `bus` by definition. So this check isn't necessary.
-
Nico Huber (Code Review)