how much more const could it be?

[ron minnich]

static const u32 const * const dual_channel_slew_group_lookup[] = {
    static const u32 const * const single_channel_slew_group_lookup[] = {
/home/rminnich/coreboot-v3/build/coreboot.initram_partiallylinked.o:
section .data.rel.ro.local: dual_channel_slew_group_lookup.3240
single_channel_slew_group_lookup.3241

what's it looking for here?

ron
p.s. const, const, const, const ==> spam, spam, spam, spam, spam,
spam, spam, spam

[Myles Watson]

On Fri, Nov 14, 2008 at 9:24 AM, ron minnich  wrote:
>    static const u32 const * const dual_channel_slew_group_lookup[] = {
>    static const u32 const * const single_channel_slew_group_lookup[] =
>
> {
> /home/rminnich/coreboot-v3/build/coreboot.initram_partiallylinked.o:
> section .data.rel.ro.local: dual_channel_slew_group_lookup.3240
> single_channel_slew_group_lookup.3241
>
> what's it looking for here?

I don't really have an answer, but I have a question:

Is it a types problem?

static const u32 nc[] = {...};
static const u32 const * const dual_channel_slew_group_lookup[] = { nc, nc
};

I think maybe it doesn't like having unconstrained pointers with this many
levels.  Could you try a typedef or something like that?  I was looking at
southbridge/amd/cs5536/cs5536.c and they don't need nearly as many consts as
you had.

I could be way off :)

Thanks,
Myles
> ron
> p.s. const, const, const, const ==> spam, spam, spam, spam, spam,
> spam, spam, spam

[ron minnich]

On Fri, Nov 14, 2008 at 9:08 AM, Myles Watson  wrote:
> On Fri, Nov 14, 2008 at 9:24 AM, ron minnich  wrote:
> >    static const u32 const * const dual_channel_slew_group_lookup[] = {
> >    static const u32 const * const single_channel_slew_group_lookup[] =
> >
> > {
> > /home/rminnich/coreboot-v3/build/coreboot.initram_partiallylinked.o:
> > section .data.rel.ro.local: dual_channel_slew_group_lookup.3240
> > single_channel_slew_group_lookup.3241
> >
> > what's it looking for here?
>
> I don't really have an answer, but I have a question:
>
> Is it a types problem?
>
> static const u32 nc[] = {...};
> static const u32 const * const dual_channel_slew_group_lookup[] = { nc, nc
> };
>
> I think maybe it doesn't like having unconstrained pointers with this many
> levels.  Could you try a typedef or something like that?  I was looking at
> southbridge/amd/cs5536/cs5536.c and they don't need nearly as many consts as
> you had.
>
> I could be way off :)

I did this:
static const u32 const dual_channel_slew_group_lookup[] = {
and cast everything in there to u32 and still no good.

ah geez. another binutils issue?

ron

[Stefan Reinauer]

ron minnich wrote:
> On Fri, Nov 14, 2008 at 9:08 AM, Myles Watson  wrote:
> > On Fri, Nov 14, 2008 at 9:24 AM, ron minnich  wrote:
> > >    static const u32 const * const dual_channel_slew_group_lookup[] = {
> > >    static const u32 const * const single_channel_slew_group_lookup[] =
> > >
> > > {
> > > /home/rminnich/coreboot-v3/build/coreboot.initram_partiallylinked.o:
> > > section .data.rel.ro.local: dual_channel_slew_group_lookup.3240
> > > single_channel_slew_group_lookup.3241
> > >
> > > what's it looking for here?
> >
> > I don't really have an answer, but I have a question:
> >
> > Is it a types problem?
> >
> > static const u32 nc[] = {...};
> > static const u32 const * const dual_channel_slew_group_lookup[] = { nc, nc
> > };
> >
> > I think maybe it doesn't like having unconstrained pointers with this many
> > levels.  Could you try a typedef or something like that?  I was looking at
> > southbridge/amd/cs5536/cs5536.c and they don't need nearly as many consts as
> > you had.
> >
> > I could be way off :)
>
> I did this:
> static const u32 const dual_channel_slew_group_lookup[] = {
> and cast everything in there to u32 and still no good.

The construct in the v2 code is correct. Don't mess with it, or it will
break.
> ah geez. another binutils issue?

What binutils version do you have? If it's lower than 2.17.x go update
to 2.18.50.x now or you will continue to have problems.

[ron minnich]

On Fri, Nov 14, 2008 at 9:49 AM, Stefan Reinauer  wrote:
> What binutils version do you have? If it's lower than 2.17.x go update
> to 2.18.50.x now or you will continue to have problems.

I have 2.18.50.x

But it is FC9 so all bets are off

ron

[Jordan Crouse]

Myles Watson wrote:
> On Fri, Nov 14, 2008 at 9:24 AM, ron minnich  wrote:
> >    static const u32 const * const dual_channel_slew_group_lookup[] = {
> >    static const u32 const * const single_channel_slew_group_lookup[] =
> >
> > {
> > /home/rminnich/coreboot-v3/build/coreboot.initram_partiallylinked.o:
> > section .data.rel.ro.local: dual_channel_slew_group_lookup.3240
> > single_channel_slew_group_lookup.3241
> >
> > what's it looking for here?

It wants to put the data in a special segment that likely isn't being 
included in the linker scripts.  We've had this problem before.  The 
linker script should be using wildcards - probably .data.rel.* or 
.data.rel.ro.* to pull in the subsections.
> I don't really have an answer, but I have a question:
>
> Is it a types problem?
>
> static const u32 nc[] = {...};
> static const u32 const * const dual_channel_slew_group_lookup[] = { nc, nc
> };
>
> I think maybe it doesn't like having unconstrained pointers with this many
> levels.  Could you try a typedef or something like that?  I was looking at
> southbridge/amd/cs5536/cs5536.c and they don't need nearly as many consts as
> you had.
>
> I could be way off :)

Hmm - I think it looks right though.  In the first line, you want the 
array to be constant, so thats correct.  In the second line, you want 
the array to be const, you want the contents to be const u32s, and you 
want the pointer itself to be a const.  So the first 'const u32' is the 
contents of the array, the second const is making the array itself 
constant, and the third const is making the pointer constant.

Jordan

[Carl-Daniel Hailfinger]

On 14.11.2008 18:41, Jordan Crouse wrote:
> Myles Watson wrote:
> > On Fri, Nov 14, 2008 at 9:24 AM, ron minnich  wrote:
> > >    static const u32 const * const
> > >
> > > dual_channel_slew_group_lookup[] = {
> > >        static const u32 const * const
> > > single_channel_slew_group_lookup[] =
> > > {
> > > /home/rminnich/coreboot-v3/build/coreboot.initram_partiallylinked.o:
> > > section .data.rel.ro.local: dual_channel_slew_group_lookup.3240
> > > single_channel_slew_group_lookup.3241
> > >
> > > what's it looking for here?
>
> It wants to put the data in a special segment that likely isn't being
> included in the linker scripts.  We've had this problem before.  The
> linker script should be using wildcards - probably .data.rel.* or
> .data.rel.ro.* to pull in the subsections.

I'm pretty sure this is a bug spotted by my section checker. .data.rel.*
must not (as in the RFC meaning) be present in initram before linking.
> > I don't really have an answer, but I have a question:
> >
> > Is it a types problem?
> >
> > static const u32 nc[] = {...};
> > static const u32 const * const dual_channel_slew_group_lookup[] = {
> > nc, nc
> > };
> >
> > I think maybe it doesn't like having unconstrained pointers with this
> > many
> > levels.  Could you try a typedef or something like that?  I was
> > looking at
> > southbridge/amd/cs5536/cs5536.c and they don't need nearly as many
> > consts as
> > you had.
> >
> > I could be way off :)
>
> Hmm - I think it looks right though.  In the first line, you want the
> array to be constant, so thats correct.  In the second line, you want
> the array to be const, you want the contents to be const u32s, and you
> want the pointer itself to be a const.  So the first 'const u32' is
> the contents of the array, the second const is making the array itself
> constant, and the third const is making the pointer constant.

Unless someone wants to implement a linker in coreboot, arrays of
pointers will be a problem in initram because initram is PIC. A possible
solution for fixed absolute pointers inside initram has been outlined in
one of my previous mails in this thread.

Regards,
Carl-Daniel

[ron minnich]

On Fri, Nov 14, 2008 at 9:41 AM, Jordan Crouse  wrote:
> Myles Watson wrote:
> > On Fri, Nov 14, 2008 at 9:24 AM, ron minnich  wrote:
> > >   static const u32 const * const dual_channel_slew_group_lookup[] = {
> > >   static const u32 const * const single_channel_slew_group_lookup[] =
> > >
> > > {
> > > /home/rminnich/coreboot-v3/build/coreboot.initram_partiallylinked.o:
> > > section .data.rel.ro.local: dual_channel_slew_group_lookup.3240
> > > single_channel_slew_group_lookup.3241
> > >
> > > what's it looking for here?
>
> It wants to put the data in a special segment that likely isn't being
> included in the linker scripts.  We've had this problem before.  The linker
> script should be using wildcards - probably .data.rel.* or .data.rel.ro.* to
> pull in the subsections.

Yes but:
  [ 8] .data.rel.ro.loca PROGBITS        00000000 004328 000600 00  WA  0   0  4

it claims it is writeable. Why is that?

ron