[PATCH] clarify << and -

[Stefan Reinauer]

Signed-off-by: Stefan Reinauer

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;

[Myles Watson]

> 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

[Stefan Reinauer]

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.

[Myles Watson]

On Wed, Apr 14, 2010 at 1:22 PM, Stefan Reinauer 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

[Carl-Daniel Hailfinger]

On 14.04.2010 21:35, Myles Watson wrote:
> On Wed, Apr 14, 2010 at 1:22 PM, Stefan Reinauer 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

[Stefan Reinauer]

On 4/14/10 9:35 PM, Myles Watson wrote:
> On Wed, Apr 14, 2010 at 1:22 PM, Stefan Reinauer
> <[e-mail] > 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.

[Myles Watson]

> 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