[U-Boot] [RFC PATCH 01/19] Introduce generic global_data

[Simon Glass]

Hi Graeme,

On Sun, Jan 8, 2012 at 2:48 AM, Graeme Russ <graeme.russ at gmail.com> wrote:
> Hi Simon,
>
> On 08/01/12 09:33, Simon Glass wrote:
>> Hi Andreas,
>>
>> On Wed, Dec 28, 2011 at 11:47 PM, Andreas Bießmann
>> <andreas.devel at googlemail.com> wrote:
>>> Dear Simon,
>>>
>>> On 28.12.11 07:35, Simon Glass wrote:
>>>> We want to unify the global_data structure. Most fields are common across
>>>> architectures, but there are a fair number of SOC-specific additions. It
>>>> isn't clear how best to deal with these, but for now we just use #ifdef.
>>>
>>
>> Sorry your email got a little buried this week.
>>
>>> I would like to see some more 'clustering' here. Especially the timer
>>> stuff is implementation specific and should therefore not coded directly
>>> but with some pointer to the specific stuff i.e. introduce some timer_t
>>> which is specific for arm in general and maybe has some more
>>> specialization for atmel devices or what else and just save a pointer to
>>> that struct in gd_t like it is done with bd_t.
>>
>> Actually I wonder if we should an architecture-specific structure,
>> something like
>>
>> struct global_data {
>>    /* generic things first */
>>    ulong baud_rate;
>>    ulong ram_size;
>>    /* hopefully lots of other things that are mostly common */
>>
>>    /* architecture-specific things */
>>    struct arch_global_data arch;
>> };
>
> This makes auto-generated asm offsets a little trickier I think
>

Yes, not impossible though.

>> struct arch_global_data  can be defined in include/asm/global_data.h
>> before it includes include/asm-generic/global_data.h, or the other way
>> around might be better.
>
> include/asm-generic/global_data.h would include include/asm/global_data.h
>
> But what of the corner case that an arch has no specific global data?
>

Define an empty struct? I would prefer not to do any of this yet
anyway, until we know that there are plenty of arch-specific things
needed.

>>
>> One thing I notice is that some architectures share some fields, and
>> others omit them.I feel we should include them at the global level if
>> at least 2 architectures need them. This means that the 'minimal'
>> architectures with hardly any fields will have quite a few unused
>> fields in global_data. I don't think that matters, and the use will
>
> I would not bank on that, and I think you will get strong objection from
> Wolfgang if you try to litter global data with members that are not
> strictly necessary

Well we have two solutions: #ifdefs and the arch-specific struct. But
the purpose of this series is rather to try to bring everything out so
we can see the commonality and check that it is correct before we
start changing it further.

>
>> increase as that arch gets more functionality.
>>
>>> Well maybe having a pointer here is stated critical so we could just
>>> insert the struct directly and let the compiler decide about the size
>>> (BTW this could be done with bd_t now too cause we have a pre-calculated
>>> size of gd_t since some time, saves some little steps when instantiating
>>> gd_t when running).
>>
>> Yes I agree that just including the structure seems better. A pointer
>> is slower and we just point to a place very close anyway.
>
> Not so fast - The idea is that gd is writeable pre-relocation and therefore
> can be instantiated in the very limit pre-SDRAM environment such as the CPU
> cache. bd is not needed until after SDRAM has been initialised, so it's
> size constraints aren't so bad.

I didn't realise this distinction. Most of the bd_info stuff seems to
be things that could just be written into the structure on demand
anyway (sort of like a call to arch_get_board_info() or similar)
rather than set up at start of day. Perhaps the heavily-used stuff
belongs in global_data. Anyway, a discussion for another day.

What is the limit on CPU cache size for SRAM in these architectures? I
assume it includes code and data, and is something like 4KB or 8KB at
worst?

>
>>
>> It would be nice to subsume bd_t into global_data.
>
> NAK

For the above reason? Well for now it is a pointer anyway.

>
>>> Beside that we could also introduce some more structs holding specific
>>> stuff i.e. console_t, reloc_t, ...
>>> (but yes, it should be done in a second step ...)
>>>
>>> What do you think about that?
>>
>> Certainly agree for reloc and perhaps there are other cases too. It is
>> far too long at present. Something to think about.
>
> I agree that gd could be cleaned up - particularly the types -
> sub-structuring it is probably a bit over the top though (as I mentioned,
> how will this impact asm-offsets?
>

We can deal with this if we need to, but first we need to see what
assembler code actually accesses it. With the effort to move
relocation etc. into C from assembler, this may reduce.

>>
>> For now I would prefer to do nothing on either point, since bringing
>> everything into one place shows up the conflicts and similarities.
>> Part of the purpose of the generic board effort is to minimise these,
>> and they are hard to spot if they are all in separate files.
>
> Hmm, I'm starting to wonder if we should instead have:
>
> struct gd_generic {
>        /* Relocation stuff */
>
>        /* pre-console stuff */
>
>        /* Jump table stuff */
> }
>
> struct gd {
>        struct gd_generic common;
>
>        /* Arch specific stuff */
> }
>
> This eliminates the 'no arch specific global data' corner case
>

Maybe. Is it best to #include <asm-generic/global_data.h> instead of
<asm/global_data.h> from U-Boot files?

>>
>> But when we get through this, and find that there are things which
>> genuinely are specific to a single arch, then I think breaking them
>> out is reasonable, hopefully combined with joining global_data and
>> bd_t.
>
> Regards,
>
> Graeme

Regards,
Simon