| Submitter | Patrick Georgi |
|---|---|
| Date | 2010-09-25 15:19:57 |
| Message ID | <4C9E131D.6090707@georgi-clan.de> |
| Download | mbox | patch |
| Permalink | /patch/1977/ |
| State | Accepted |
| Commit | r5846 |
| Headers | show |
Comments
On 9/25/10 5:19 PM, Patrick Georgi wrote: > Am 25.09.2010 16:40, schrieb repository service: >> Change Log: >> Make globals in romstage break the build, so we don't have to >> wonder why variables in .data or .bss (both somewhere in ROM space) >> are wrong. > Instant proof that this test is useful :-) I was wondering if that would happen. :-) Very nice! > Digging through these boards, the culprit is > static u8 swaplist[] = { 0xFF, CONFIG_HT_CHAIN_UNITID_BASE, > CONFIG_HT_CHAIN_END_UNITID_BASE, 0xFF }; in > src/northbridge/amd/amdht/ht_wrapper.c's > static BOOL AMD_CB_ManualBUIDSwapList (u8 node, u8 link, u8 **List). > > This is put in .data, as it might be modified by some other user of the > pointer. As far as I can see, it isn't, so that choice is harmless. > > I made it const, as well as its users, and the tree compiles, but > feedback on the approach of the patch and/or testing is _very_ welcome. > > Signed-off-by: Patrick Georgi <patrick.georgi@coresystems.de> Acked-by: Stefan Reinauer <stepan@coresystems.de>
On Sat, Sep 25, 2010 at 11:15 AM, Stefan Reinauer <stefan.reinauer@coresystems.de> wrote: > On 9/25/10 5:19 PM, Patrick Georgi wrote: >> Am 25.09.2010 16:40, schrieb repository service: >>> Change Log: >>> Make globals in romstage break the build, so we don't have to >>> wonder why variables in .data or .bss (both somewhere in ROM space) >>> are wrong. >> Instant proof that this test is useful :-) > I was wondering if that would happen. :-) Very nice! > >> Digging through these boards, the culprit is >> static u8 swaplist[] = { 0xFF, CONFIG_HT_CHAIN_UNITID_BASE, >> CONFIG_HT_CHAIN_END_UNITID_BASE, 0xFF }; in >> src/northbridge/amd/amdht/ht_wrapper.c's >> static BOOL AMD_CB_ManualBUIDSwapList (u8 node, u8 link, u8 **List). >> >> This is put in .data, as it might be modified by some other user of the >> pointer. As far as I can see, it isn't, so that choice is harmless. >> >> I made it const, as well as its users, and the tree compiles, but >> feedback on the approach of the patch and/or testing is _very_ welcome. >> >> Signed-off-by: Patrick Georgi <patrick.georgi@coresystems.de> > Acked-by: Stefan Reinauer <stepan@coresystems.de> That should be fine and shouldn't break anything. It is a buildtime setting. Marc
Patch
Index: src/northbridge/amd/amdht/h3finit.c =================================================================== --- src/northbridge/amd/amdht/h3finit.c (Revision 5841) +++ src/northbridge/amd/amdht/h3finit.c (Arbeitskopie) @@ -899,7 +899,7 @@ u32 unitIDcnt; SBDFO currentPtr; u8 depth; - u8 *pSwapPtr; + const u8 *pSwapPtr; SBDFO lastSBDFO = ILLEGAL_SBDFO; u8 lastLink = 0; Index: src/northbridge/amd/amdht/h3finit.h =================================================================== --- src/northbridge/amd/amdht/h3finit.h (Revision 5841) +++ src/northbridge/amd/amdht/h3finit.h (Arbeitskopie) @@ -195,7 +195,7 @@ * * --------------------------------------------------------------------------------------- */ - BOOL (*AMD_CB_ManualBUIDSwapList)(u8 Node, u8 Link, u8 **List); + BOOL (*AMD_CB_ManualBUIDSwapList)(u8 Node, u8 Link, const u8 **List); /**---------------------------------------------------------------------------------------- * Index: src/northbridge/amd/amdht/ht_wrapper.c =================================================================== --- src/northbridge/amd/amdht/ht_wrapper.c (Revision 5841) +++ src/northbridge/amd/amdht/ht_wrapper.c (Arbeitskopie) @@ -117,9 +117,9 @@ * @param[out] BOOL result = true to use a manual list * false to initialize the link automatically */ -static BOOL AMD_CB_ManualBUIDSwapList (u8 node, u8 link, u8 **List) +static BOOL AMD_CB_ManualBUIDSwapList (u8 node, u8 link, const u8 **List) { - static u8 swaplist[] = { 0xFF, CONFIG_HT_CHAIN_UNITID_BASE, CONFIG_HT_CHAIN_END_UNITID_BASE, 0xFF }; + static const u8 swaplist[] = { 0xFF, CONFIG_HT_CHAIN_UNITID_BASE, CONFIG_HT_CHAIN_END_UNITID_BASE, 0xFF }; /* If the BUID was adjusted in early_ht we need to do the manual override */ if ((CONFIG_HT_CHAIN_UNITID_BASE != 0) && (CONFIG_HT_CHAIN_END_UNITID_BASE != 0)) { printk(BIOS_DEBUG, "AMD_CB_ManualBUIDSwapList()\n");