Hi Kim,
On 17.09.2010 03:54, Kim Phillips wrote:
>> +#if defined(CONFIG_SYS_PCIE2_CFG_BASE)&& defined(CONFIG_SYS_PCIE2_SIZE)
>> #define PCIE_MAX_BUSES 2
>> +#else
>> +#define PCIE_MAX_BUSES 1
>> +#endif
>>
> Technically this should be an indirect function of MPC8308, but what's
> the problem with leaving MAX_BUSES as 2 always?
>
Well, we can save a couple of bytes but I agree it doesn't worth it.
I'll remove the ifdef here.
>> + unsigned int *cfg_addr;
>> +
>> + if (bus == 0) {
>> + cfg_addr = (unsigned int *)CONFIG_SYS_PCIE1_CFG_BASE;
>> + } else {
>> +#if defined(CONFIG_SYS_PCIE2_CFG_BASE)&& defined(CONFIG_SYS_PCIE2_SIZE)
>> + cfg_addr = (unsigned int *)CONFIG_SYS_PCIE2_CFG_BASE;
>> +#else
>> + printf("Second PCIE host controller not configured!\n");
>> + return;
>> +#endif
>>
> let's reduce the new ifdefs introduced in the file down to one, and
> still be able remove the 'if (bus)' clauses by adding a static pcie cfg
> struct array[], to which this and code below it would directly
> dereference with array[bus]. The single ifdef should protect the
> second entry in the array. The "second PCIE host controller not
> configured" check can be made by comparing num_buses with ARRAY_SIZE
> (array) in mpc83xx_pcie_init prior to calling mpc83xx_pcie_init_bus
> with an illegal bus value.
>
Yes, this sounds like much cleaner code. I'll post the updated patches
in the short while.
Regards, Ilya.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
