Re: [PATCH v2 1/1] powerpc/86xx: Add support for Emerson/Artesyn MVME7100

[Alessio Igor Bogani]

Hi Scott,

Thanks for reviewing it!

On 19 April 2016 at 06:26, Scott Wood <oss@xxxxxxxxxxxx> wrote:
> On Mon, 2016-04-18 at 09:57 +0200, Alessio Igor Bogani wrote:
>> +     pci0: pcie@f1008000 {
>> +             reg = <0xf1008000 0x1000>;
>> +             ranges = <0x02000000 0x0 0x80000000 0x80000000 0x0
>> 0x50000000
>> +                       0x01000000 0x0 0x00000000 0xf0000000 0x0
>> 0x00800000>;
[...]
>> +
>> +     pci1: pcie@f1009000 {
>> +             compatible = "fsl,mpc8641-pcie";
>> +             device_type = "pci";
>> +             #size-cells = <2>;
>> +             #address-cells = <3>;
>> +             reg = <0xf1009000 0x1000>;
>> +             bus-range = <0 0xff>;
>
> Why are pci0 and pci1 so different? Why does mpc8641si-post.dtsi not have pci1?

You are right. The MPC8641 processor offers two pci so
mpc8641si-post.dtsi should be the right place where to define both.
What about the boards which don't use the pci1? Will 'status =
"disabled"' be enough?

>> +asm("        .globl _zimage_start\n\
>> +     _zimage_start:\n\
>> +             mfmsr   10\n\
>> +             rlwinm  10,10,0,~(1<<15)        /* Clear MSR_EE */\n\
>> +             sync\n\
>> +             mtmsr   10\n\
>> +             isync\n\
>> +             b _zimage_start_lib\n\
>> +");
>
> Please put this in an asm file.

Ok.

> Is U-Boot really not clearing MSR[EE]?  How old is this U-Boot?
[...]
>> +     defined(TARGET_83xx) || defined(TARGET_MVME7100)
>>       unsigned long   bi_immr_base;   /* base of IMMR register
[...]
> TARGET_86xx would match the U-Boot definition better.
[...]
>> +     _set_L2CR(_get_L2CR() | L2CR_L2E);
[...]
> U-Boot doesn't enable L2 cache?

In fact it isn't U-Boot but MotLoad which it doesn't clear MSR[EE] and
disable L2 cache just before pass control to operating system. It
seems to be partially compatible with older version of U-Boot
(boot/mvme7100.c) but I wasn't able to use an uImage to boot so I have
switched to the dtbImage target.

Thanks!

Ciao,
Alessio