[U-Boot] [PATCH 3/6] P1021: add P1021MDS board support

[Haiying Wang]

On Mon, 2011-01-31 at 21:03 +0100, Wolfgang Denk wrote:
> Dear Haiying.Wang at freescale.com,
> > diff --git a/board/freescale/p1021mds/config.mk b/board/freescale/p1021mds/config.mk
> > new file mode 100644
> > index 0000000..3888f61
> > --- /dev/null
> > +++ b/board/freescale/p1021mds/config.mk
> ...
> > +ifndef NAND_SPL
> > +ifndef IN_TPL
> > +ifeq ($(CONFIG_NAND), y)
> > +LDSCRIPT := $(TOPDIR)/$(CPUDIR)/u-boot-nand.lds
> > +endif
> > +endif
> > +endif
> 
> Why is this config.mk needed?  Can you not do all this in the board
> config file instead?
Do you mean the board header file or arch/powerpc/config.mk? I did not see any LDSCRIPT defined in Freescale board header file.

> > diff --git a/board/freescale/p1021mds/ddr.c b/board/freescale/p1021mds/ddr.c
> > new file mode 100644
> > index 0000000..594a4a8
> > --- /dev/null
> > +++ b/board/freescale/p1021mds/ddr.c
> 
> It seems there are a number of functions here which ar actually shared
> with other files, for example board/freescale/p1022ds/ddr.c.
Every boards has its board specific ddr parameters which are defined the its own board ddr.c. The common code for ddr has been defined in arch/powerpc/cpu/mpc8xxx/ddr/.

> I wonder if it is not possible to use more common code here - especially
> given the fact that we already have a nice collection of such files:
> 
> 	board/freescale/corenet_ds/ddr.c
> 	board/freescale/mpc8536ds/ddr.c
> 	board/freescale/mpc8540ads/ddr.c
> 	board/freescale/mpc8541cds/ddr.c
> 	board/freescale/mpc8544ds/ddr.c
> 	board/freescale/mpc8548cds/ddr.c
> 	board/freescale/mpc8555cds/ddr.c
> 	board/freescale/mpc8560ads/ddr.c
> 	board/freescale/mpc8568mds/ddr.c
> 	board/freescale/mpc8569mds/ddr.c
> 	board/freescale/mpc8572ds/ddr.c
> 	board/freescale/mpc8610hpcd/ddr.c
> 	board/freescale/mpc8641hpcn/ddr.c
> 	board/freescale/p1022ds/ddr.c
> 	board/freescale/p1_p2_rdb/ddr.c
> 	board/freescale/p2020ds/ddr.c
If you go to see each ddr.c, you can find there is
fsl_ddr_board_options() which defines the different values for each
board. Also fsl_ddr_get_spd() is also highly dependent on board, like
ddr type(ddr2 or ddr3), i2c spd eeprom address, ddr controller# etc.
Only  fsl_ddr_get_mem_data_rate()might be moved to common code.

> > +void board_lmb_reserve(struct lmb *lmb)
> > +{
> > +	cpu_mp_lmb_reserve(lmb);
> > +}
> 
> How many board/freescale/<name>/<name>.c file share this same code?
There are some, but I don't know whether there will be difference coming in later.

> 
> > diff --git a/board/freescale/p1021mds/tlb.c b/board/freescale/p1021mds/tlb.c
> > new file mode 100644
> > index 0000000..30af6dd
> > --- /dev/null
> > +++ b/board/freescale/p1021mds/tlb.c
> 
> How much of this is actually different from - say -
> board/freescale/p1022ds/tlb.c ?
The tlb.c is also a highly board dependent file. Different boards have different supported peripherals. If you look at p1021 and p1022's tlb.c files, you can see p1022ds has 3 PCIE, P1021 has 2, P1022ds has NOR flash, P1021MDS only has NAND flash... etc.


Haiying