Attention is currently required from: Angel Pons, Arthur Heymans, Felix Held, Krystian Hebel, Kyösti Mälkki, Nico Huber.
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 16:
(5 comments)
File src/device/pciexp_device.c:
https://review.coreboot.org/c/coreboot/+/77338/comment/c056e7a8_dceb4526 : PS15, Line 638: pciexp_dev_set_max_payload_size(child, max_payload);
Shouldn't be necessary as that's what we'll do with pciexp_sync_max_payload_size().
Indeed, we only care about the MPS propagation upwards. Removed.
https://review.coreboot.org/c/coreboot/+/77338/comment/53dc2160_b2d34bd3 : PS15, Line 641: if (max_payload != child_max_payload) : printk(BIOS_INFO, "%s: Max_Payload_Size adjusted to %d\n", dev_path(child), : (1 << (max_payload + 7)));
This too could be dropped then.
Removed.
https://review.coreboot.org/c/coreboot/+/77338/comment/b5d8daf6_0fa66bbd : PS15, Line 691: };
I still don't see why we would need a loop. pciexp_tune_dev() will be called […]
The whole function is not needed at all actually. AS you have pointed out, we already set the MPS recursively upwards.
https://review.coreboot.org/c/coreboot/+/77338/comment/448c3553_587c5a2c : PS15, Line 728: pciexp_dev_set_max_payload_size(dev, pciexp_dev_get_max_payload_size_cap(dev));
If I'm not mistaken, `dev` could be a bridge that already has its […]
Yes, limited it to express endpoints and legacy endpoints.
https://review.coreboot.org/c/coreboot/+/77338/comment/4fc6d068_b486ca15 : PS15, Line 740: pciexp_dev_set_max_payload_size(bus->dev, max_payload);
This step seems redundant. Because for the first call of […]
Right, not sure what I was thinking... Removed.