[U-Boot-Users] ppc4xx: [PATCH] CPU PPC440x5 on Virtex5 FX (new version)

[Ricardo Ribalda Delgado]

Hello Stefan

   Thanks for your comments.



On Thu, Jul 17, 2008 at 8:24 AM, Stefan Roese <sr at denx.de> wrote:

> Even though we usually sort the files alphabetically, these
> multiple "ifndef's" look very ugly. Perhaps we should change this here to a
> single "ifndef" with all the non-Xilinx files in it.

OK, done

>
> Is the CPU really a VIRTEX5 or a PPC440 variant? I think the latter. Perhaps
> something like "PPC440x5"?

The CPU is a PPC440x5 by IBM designed for the Virtex5, I think that is
more descriptive Virtex5 than just x5. What do you thing about:
      puts ("x5 VIRTEX5");

>
> Nitpick: Only one empty line please.
:) Sorry, done

>
> Do you really need this in this file?

Yes, is the external interrupt handler of the CPU, it is needed for
other elemments like the ethernet and the i2c.

> Don't put the casts into the defines. They belong in the code.

Done


> Conding-style: Missing spaces.

Done

>
> You probably what a debug() here.

I liked the printf :) but I have changed it to a debug

>
> I personally prefer to use 0xffffffff here.
OK


> I suggest you decide for one type here and not use "unsigned int"
> and "unsigned long". I use u32 now if possible. This is for the whole file.

Done


> Please move those defines into some interrupt controller specific headers. For
> uic version this is include/asm-ppc/ppc4xx-uic.h.
>

Done

> Best regards,
> Stefan
>
> =====================================================================
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
> =====================================================================
>



-- 
Ricardo Ribalda
http://www.eps.uam.es/~rribalda/