[U-Boot] [PATCH v4 02/12] SPEAr : Adding basic SPEAr architecture support.

[Tom]

Vipin Kumar wrote:
> Hello Tom,
> 
>> +
>> +#include <common.h>
>> +#include <asm/io.h>
>> +#include <asm/arch/spr_syscntl.h>
>> +
>> +void reset_cpu(ulong ignored)
>> +{
>> +       struct syscntl_regs *syscntl_regs_p =
>> +           (struct syscntl_regs *)CONFIG_SPEAR_SYSCNTLBASE;
>> +
>> This is not bi-sect able
>>
>> CONFIG_SPEAR_SYSCNTLBASE is not defined until #7.
>> This looks like a register base address.
>> This should be defined in spr_syscntl.h
>>
>> This also applies to CONFIG_SPEAR_MISCBASE.
>> Should be in spr_misc.h
>>
> 
> I intentionally used include/configs/spear.h as the location to define
> base addresses. The reason is that same IP can be embedded in more
> than one SoCs. Offcourse, the IP can lie at a different base address
> in each of them. In that case,we pick base address from
> include/configs/spear.h
> 
> Also, other architectures use configs/xxx.h for defining base addresses
> 
> Now, because spear.h is not added until the support for spear600
> board is added, it leads to the problem you described above.
> Offcourse, a build is not possible before adding first board support
> 
> So how should we prcoceed,
>  1. keep it as it is bacause build in not possible OR
No

>  2. reorder the patch
I do not think simple reording of the patches ok either.
As I want the SOC support first, then boards.

You are using the spear.h file as if it was in include/asm/arch/spear.h
not in include/configs.

There are other problems with spear.h
At the least it will need to be a version of this per board.

To be bisectable.
1. You need to define register offsets in asm/arch file or
2. Move the non bisectable code to a later patch

I prefer 1.

Tom

> 
> Best Regards,
> Vipin