[U-Boot] [PATCH v3 08/28] Introduce generic global_data

[Simon Glass]

Hi Mike,

On Mon, Mar 5, 2012 at 9:01 PM, Mike Frysinger <vapier at gentoo.org> wrote:
> On Thursday 16 February 2012 09:48:55 Simon Glass wrote:
>> --- /dev/null
>> +++ b/include/asm-generic/global_data.h
>>
>> +typedef struct global_data {
>> +     bd_t            *bd;
>> +     unsigned long   flags;
>> +     unsigned long   baudrate;
>> +#if defined(CONFIG_LCD) || defined(CONFIG_VIDEO)
>> +     unsigned long   fb_base;        /* Base address of framebuffer mem */
>> +#endif
>> +#if defined(CONFIG_POST) || defined(CONFIG_LOGBUFFER)
>> +     unsigned long   post_log_word;  /* Record POST activities */
>> +     unsigned long   post_log_res; /* success of POST test */
>> +     unsigned long   post_init_f_time;  /* When post_init_f started */
>> +#endif
>> +     unsigned long   have_console;   /* serial_init() was called */
>> +#ifdef CONFIG_PRE_CONSOLE_BUFFER
>> +     unsigned long   precon_buf_idx; /* Pre-Console buffer index */
>> +#endif
>> +     unsigned long   env_addr;       /* Address  of Environment struct */
>> +     unsigned long   env_valid;      /* Checksum of Environment valid? */
>> +     /* Here begins ARM-specific things. Needs discussion */
>
> yes, as soon as the "generic" header has soc/arch/board specific cruft, it's
> failed.  some of these fields most likely are unnecessary (either in gd_t or at
> all), so those should get moved/dropped.  for the ones that really truly need
> to be in global data, i don't see the problem with having an soc-specific sub
> field.

I think that's a little harsh. After all it's a bit much to expect a
unified board setup in a single stroke. We need to weed out the
differences between architectures, which is much easier if these
differences are all in one file. So I would not say 'failed', but just
'stepping stone along the way'. These fields stand out like a sore
thumb which is the idea. Many will in fact be common and the #ifdefs
can be removed. Others may be only for one architecture but with
strong justification, so we may make them separate.

If we want an soc-specific sub-field then I see that as a follow-on
series for each field or group of fields, and each being justified by
a pretty strong case that the concept only exists in that one arch.
Personally I would rather we avoid it as long as possible - it is
exactly what I am trying to remove.

>
> it should be:
>        include/global_data.h:
>        #include <asm/global_data.h>
>        typedef struct global_data {
>                arch_gd_t arch_gd;
>                ...
>
> and then the arch can put the stuff that they truly need early in the boot
> sequence into arch_gd_t.

Well yes, but then we lose the idea of a unified global data
structure. Archs will be free to put whatever they like in there and
we will be back to where we are now. Can we not go through a process
of weeding out the unneeded things in this structure, and thinking
very hard before we allow per-architecture fields? I see this effort
as starting with generic board being available, and then gathering
pace as more boards switch to it and we rationalise things.

>
>> +/*
>> + * Global Data Flags
>> + */
>
> i posted a patch to unify this

Yes I think I saw that - how should we handle this? Should I rebase on
top of it?

Regards,
Simon

> -mike