[U-Boot] [RFC] RFC: convert MPC8536DS to use generic board

[York Sun]

On 04/26/2014 02:22 AM, Wolfgang Denk wrote:
> Dear York Sun,
> 
> In message <1398474623-4709-1-git-send-email-yorksun at freescale.com> you wrote:
>>
>> Add #ifdef CONFIG_OF_CONTROL for reserve_fdt(), setup_fdt(), reloc_fdt().
> 
> This looks wrong to me.  This is a global file, and you are affecting
> a ton of unrelated boards.

Understood it is a global file. These functions deal with FDT. They should not
be in the path if targets don't use device tree to configure their devices. If
there is another more appropriate macro to use, please let me know. Take the
example I used, MPC8536DS, the gd->fdt_blob would have incorrect value because
it doesn't use device tree.

> 
>> Set initial value for gd. Powerpc SoCs use locked cache as init RAM.
> 
> Well, some of them do, not all.

arch/powerpc/lib/board.c uses this way. I presume it is safe to use for all PPC
parts.nn

> 
>> Change return value for mac_read_from_eeprom() when mismatch happens to
>> prevent calling hang().
> 
> You mean, you just ignore the error?  This is a change of the cpolicy
> that has nothing to do with generic board support, right?  Why should
> this be done now, i. e. why has it been accepted and considered to be
> working before?

This function is helpful but not critical. If reading fails, the board should
continue to boot then users will have a chance to fix it. The new generic board
treats this as other functions in board_init_r. Any error will cause hanging.


> 
>>  board/freescale/common/sys_eeprom.c |    2 +-
>>  common/board_f.c                    |   18 +++++++++++++++++-
>>  include/configs/MPC8536DS.h         |    2 ++
>>  3 files changed, 20 insertions(+), 2 deletions(-)
> 
> I think thease are at least 2, eventually 3 independent changes.  You
> should split them in several commits.

Again, this patch is for discussion. Once we are clear what we should fix, I
will generate appropriate patch set.

> 
>> +#ifdef CONFIG_PPC
>> +	gd = (gd_t *) (CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_GBL_DATA_OFFSET);
>> +	__asm__ __volatile__("":::"memory");
>> +#endif
> 
> Again, this is a global change.  Why is this now needed?
> 

It has been this way for powerpc. Do we have an alternative?

York