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

Ricardo Ribalda Delgado
2008-07-17T09:22:29Z

Hello Stefan

Thanks for your comments.

On Thu, Jul 17, 2008 at 8:24 AM, Stefan Roese <sr@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