On Wed, May 25, 2011 at 1:59 PM, Jon Mason <jon.mason@xxxxxxxx> wrote: > On Wed, May 25, 2011 at 2:34 PM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote: >> On Wed, May 25, 2011 at 1:01 PM, Jon Mason <jon.mason@xxxxxxxx> wrote: >>> Ensure that the max payload size on the root port is the same as the max >>> payload size on all intermediate bridges and devices. This is required >>> to work around buggy BIOS revisions found on various whitebox >>> motherboards which do not configure mps beyond one level below the root >>> port. >>> >>> Signed-off-by: Jon Mason <mason@xxxxxxxx> >>> Signed-off-by: Andrew Gallatin <gallatin@xxxxxxxx> >>> --- >>> drivers/pci/bus.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ >>> 1 files changed, 44 insertions(+), 0 deletions(-) >>> >>> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c >>> index 69546e9..fc16b67 100644 >>> --- a/drivers/pci/bus.c >>> +++ b/drivers/pci/bus.c >>> @@ -175,6 +175,49 @@ int pci_bus_add_child(struct pci_bus *bus) >>> return retval; >>> } >>> >>> +/* >>> + * Ensure that the max payload size on the root port is the same as the max >>> + * payload size on all intermediate bridges. >> >> This description doesn't seem to quite match what the function does. >> It looks like the function sets a single PCIe device's MPS to that of >> the upstream bridge. > > Does this sound better? > > /* > * This is required to work around buggy BIOS revisions found on various > * whitebox motherboards which do not configure mps beyond one level below the > * root port. Ensure that the max payload size of all children bridges and > * devices are the same as the parent bus. Yep, I think that makes more sense. I do think the "all ... bridges and device" part is still a bit of an overstatement when describing this function all by itself, since it operates on a single device. The "all" part comes from where you put the caller, and in my mind, that's not important enough for its own comment. I'm carefully not saying anything about the actual content of the patch because I don't know how MPS works or what the constraints are. For example, I don't know whether it's legal for a child to have a *smaller* MPS than an upstream bridge. What happens when a device is hot-added? Is it sufficient to simply propagate the upstream bridge MPS values down to the new device, or is it possible that MPS values in the rest of the fabric need to be adjusted, too? Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
