Dear Wolfgang, thanks for your review.
On 21.06.2010 11:44, Wolfgang Denk wrote: >> This patch adds basic support for Freescale MPC8308 CPU. Serial ports, >> NOR flash and integrated Ethernet controllers are supported. >> PCI Express is also supported. eSDHC, NAND and USB may work but aren't >> tested (using ULPI PHY requires additional patch). >> >> Signed-off-by: Ilya Yanok<ya...@emcraft.com> >> > ... > >> -#if defined(CONFIG_MPC834x) || defined(CONFIG_MPC837x) || >> defined(CONFIG_MPC8315) >> +#if defined(CONFIG_MPC834x) || defined(CONFIG_MPC837x) || \ >> + defined(CONFIG_MPC8315) || defined(CONFIG_MPC8308) >> > Please sort this list. > Fixed. >> +#if defined(CONFIG_MPC8308) >> +#define SCCR_SDHCCM 0x0c000000 >> +#define SCCR_SDHCCM_SHIFT 26 >> +#define SCCR_SDHCCM_0 0x00000000 >> +#define SCCR_SDHCCM_1 0x04000000 >> +#define SCCR_SDHCCM_2 0x08000000 >> +#define SCCR_SDHCCM_3 0x0c000000 >> +#endif >> > Would it make sense to write this as: > > And: why do we need the #ifdef? Unused defines should not hurt? > > #define SCCR_SDHCCM_MASK 0x0c000000 /* is it a mask? */ > #define SCCR_SDHCCM_SHIFT 26 > #define SCCR_SDHCCM(arg) ((arg)<<SCCR_SDHCCM_SHIFT) > > As you already mentioned I'm just following the style used in this file. >> +#if defined(CONFIG_MPC8315) >> #define SCCR_SATA1CM 0x00003000 >> #define SCCR_SATA1CM_SHIFT 12 >> #define SCCR_SATACM 0x00003c00 >> @@ -765,6 +776,7 @@ >> #define SCCR_SATACM_1 0x00001400 >> #define SCCR_SATACM_2 0x00002800 >> #define SCCR_SATACM_3 0x00003c00 >> +#endif >> > Do we need that #ifdef? Ok, the #defines don't apply to the 8308, but > do they hurt if they are just there, unused? > Well, it seems to be safer not to have unused defines so that you can't erroneously use some define not applicable for current CPU, but if you wish I'll remove these ifdefs. Regards, Ilya. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
