| Submitter | Stefan Reinauer |
|---|---|
| Date | 2010-04-14 15:47:31 |
| Message ID | <4BC5E393.9050405@coresystems.de> |
| Download | mbox | patch |
| Permalink | /patch/1235/ |
| State | Rejected |
| Headers | show |
Comments
> Index: src/northbridge/amd/gx2/chipsetinit.c > =================================================================== > --- src/northbridge/amd/gx2/chipsetinit.c (revision 5425) > +++ src/northbridge/amd/gx2/chipsetinit.c (working copy) > @@ -275,7 +275,7 @@ > if ((msr.lo&0xff) == 0x11) > return; > > - totalmem = sizeram() << 20 - 1; > + totalmem = (sizeram() << 20) - 1; > I agree that it looks right, but it changes the answer, since subtraction has a higher precedence than shift. I don't have a board to test which is right. Did you check v3? Thanks, Myles
On 4/14/10 6:44 PM, Myles Watson wrote: > > Index: src/northbridge/amd/gx2/chipsetinit.c > =================================================================== > --- src/northbridge/amd/gx2/chipsetinit.c (revision 5425) > +++ src/northbridge/amd/gx2/chipsetinit.c (working copy) > @@ -275,7 +275,7 @@ > if ((msr.lo&0xff) == 0x11) > return; > > - totalmem = sizeram() << 20 - 1; > + totalmem = (sizeram() << 20) - 1; > > I agree that it looks right, but it changes the answer, since > subtraction has a higher precedence than shift. > > I don't have a board to test which is right. Did you check v3? Nor do I... v3 never supported gx2.
On Wed, Apr 14, 2010 at 1:22 PM, Stefan Reinauer <stepan@coresystems.de>wrote: > On 4/14/10 6:44 PM, Myles Watson wrote: > > > Index: src/northbridge/amd/gx2/chipsetinit.c >> =================================================================== >> --- src/northbridge/amd/gx2/chipsetinit.c (revision 5425) >> +++ src/northbridge/amd/gx2/chipsetinit.c (working copy) >> @@ -275,7 +275,7 @@ >> if ((msr.lo&0xff) == 0x11) >> return; >> >> - totalmem = sizeram() << 20 - 1; >> + totalmem = (sizeram() << 20) - 1; >> > I agree that it looks right, but it changes the answer, since subtraction > has a higher precedence than shift. > > I don't have a board to test which is right. Did you check v3? > > Nor do I... > > v3 never supported gx2. > I guess this should be the patch, then: - totalmem = sizeram() << 20 - 1; + totalmem = sizeram() << (20 - 1); Anyone with an OLPC board care to chime in? Thanks, Myles
On 14.04.2010 21:35, Myles Watson wrote: > On Wed, Apr 14, 2010 at 1:22 PM, Stefan Reinauer <stepan@coresystems.de>wrote: > > >> On 4/14/10 6:44 PM, Myles Watson wrote: >> >> >> Index: src/northbridge/amd/gx2/chipsetinit.c >> >>> =================================================================== >>> --- src/northbridge/amd/gx2/chipsetinit.c (revision 5425) >>> +++ src/northbridge/amd/gx2/chipsetinit.c (working copy) >>> @@ -275,7 +275,7 @@ >>> if ((msr.lo&0xff) == 0x11) >>> return; >>> >>> - totalmem = sizeram() << 20 - 1; >>> + totalmem = (sizeram() << 20) - 1; >>> >>> >> I agree that it looks right, but it changes the answer, since subtraction >> has a higher precedence than shift. >> >> I don't have a board to test which is right. Did you check v3? >> >> Nor do I... >> >> v3 never supported gx2. >> >> > > I guess this should be the patch, then: > > - totalmem = sizeram() << 20 - 1; > + totalmem = sizeram() << (20 - 1); > > Anyone with an OLPC board care to chime in? > I won't be able to test before April 20. If that's still to be tested by then, I'd appreciate a reminder. Regards, Carl-Daniel
On 4/14/10 9:35 PM, Myles Watson wrote: > > > On Wed, Apr 14, 2010 at 1:22 PM, Stefan Reinauer > <stepan@coresystems.de <mailto:stepan@coresystems.de>> wrote: > > On 4/14/10 6:44 PM, Myles Watson wrote: >> >> Index: src/northbridge/amd/gx2/chipsetinit.c >> =================================================================== >> --- src/northbridge/amd/gx2/chipsetinit.c (revision 5425) >> +++ src/northbridge/amd/gx2/chipsetinit.c (working copy) >> @@ -275,7 +275,7 @@ >> if ((msr.lo&0xff) == 0x11) >> return; >> >> - totalmem = sizeram() << 20 - 1; >> + totalmem = (sizeram() << 20) - 1; >> >> I agree that it looks right, but it changes the answer, since >> subtraction has a higher precedence than shift. >> >> I don't have a board to test which is right. Did you check v3? > Nor do I... > > v3 never supported gx2. > > > I guess this should be the patch, then: > > - totalmem = sizeram() << 20 - 1; > + totalmem = sizeram() << (20 - 1); If we can't decide on the right fix we should leave the warning there as it keeps the question open until someone can answer it.
> > If we can't decide on the right fix we should leave the warning there as it > keeps the question open until someone can answer it. > I agree. Thanks, Myles
Patch
Index: src/northbridge/amd/gx2/chipsetinit.c =================================================================== --- src/northbridge/amd/gx2/chipsetinit.c (revision 5425) +++ src/northbridge/amd/gx2/chipsetinit.c (working copy) @@ -275,7 +275,7 @@ if ((msr.lo&0xff) == 0x11) return; - totalmem = sizeram() << 20 - 1; + totalmem = (sizeram() << 20) - 1; totalmem >>= 12; totalmem = ~totalmem; totalmem &= 0xfffff;
Signed-off-by: Stefan Reinauer <stepan@coresystems.de>