Patch set 2:Code-Review +2

View Change

1 comment:

• File src/northbridge/intel/ironlake/northbridge.c:

  • Patch Set #2, Line 137: it uncacheable, though, for easier MTRR allocation. */

    "Warning: coreboot did what its code says, everything is alright"? ;) […]

    What bugs me is that this only seems cause problems now. How did it work in the past? Maybe the old allocator did something different, even if it was because of a bug. Thus, I would prefer if the logs would have clear indications of known potential problems. For the message, I'd use something like:

     printk(BIOS_NOTICE, "Reserving unexpected region between top of TSEG and GTT base.\n");

    I don't have a strong opinion on this, though. I wouldn't care at all if I had a reliable reference to verify raminit for this platform, but alas.

    This is the same reasoning that led me to make CB:43778 (which I abandoned because it got stalled by review comments, and made CB:44155 which also actually fixes the problem in the meantime). There was no clear indication that `root_port_commit_config` had been called. If there had been such a message from the beginning, it would have been much easier to find the problem. Which CB:30077 could have added, btw.

To view, visit change 45325. To unsubscribe, or for help writing mail filters, visit settings.