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

[Wolfgang Denk]

Dear Haiying Wang,

In message <1296509955.2049.543.camel at haiying-laptop> you wrote:
>
> > 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.

I mean the board config header file, include/configs/<name>.h

> > > 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/.

Well, but there is tons of common code. For example, all of

	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/p1021mds/ddr.c
	board/freescale/p1022ds/ddr.c
	board/freescale/p2020ds/ddr.c

share the same function

	unsigned int fsl_ddr_get_mem_data_rate(void)
	{
		return get_ddr_freq(0);
	}

And

	board/freescale/p1021mds/ddr.c
	board/freescale/p1022ds/ddr.c

share (except for the comment) the same

	void fsl_ddr_get_spd(ddr3_spd_eeprom_t *ctrl_dimms_spd, unsigned int ctrl_num)

while
	board/freescale/corenet_ds/ddr.c
	board/freescale/mpc8569mds/ddr.c

use another variant, but again both boards the same one.


> 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.

Actually this is not quite true. See examples above.

> > > +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.

Then we can use a common implementation for all where it fits, and
use board specific code only where needed.

> > > 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

Yes, there are differences. But it seems there is more common code
than differing one?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A penny saved is a penny to squander.
- Ambrose Bierce