Re: [U-Boot] [PATCH 4/7] powerpc/p1021: Add P1021MDS board support

2010-08-17 Thread Wolfgang Denk 
Dear Haiying Wang,

In message <1282025889.2814.80.ca...@localhost.localdomain> you wrote:
>
> > Please avoid adding a new header file just for this single prototype.
> This was copied from mpc8568mds/mpc8569mds. If it is not allowed anymore, I 
> can remove it.

Please do.

Ideally provide a cleanup patch for mpc8569mds as well.

> > Why do you need a separate one anyway? 
> Because it is p1021mds board specific reset routine.

Yes, what is the problem> We have support for board specific code
here, don't we?

> > Why cannot you implement this
> > in reset_phy() ?
> reset_phy() is a one time reset and called after eth_initialize(). But
> the board designer told me to reset phy for each UEC port before
> initializing it, otherwise the phy can not work properly.

Inded this needs to be reworked. Please wait a few days, there might
be a patch coming.

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: w...@denx.de
"We shall reach greater and greater platitudes of achievement."
- Richard J. Daley
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 4/7] powerpc/p1021: Add P1021MDS board support

2010-08-16 Thread Haiying Wang 
On Mon, 2010-16-08 at 12:33 +0200, Wolfgang Denk wrote:
> > --- /dev/null
> > +++ b/board/freescale/p1021mds/bcsr.h
> > @@ -0,0 +1,18 @@
> > +/*
> > + * Copyright (C) 2010 Freescale Semiconductor, Inc.
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License as published by the 
> > Free
> > + * Software Foundation; either version 2 of the License, or (at your 
> > option)
> > + * any later version.
> > + *
> > + */
> > +
> > +#ifndef __BCSR_H_
> > +#define __BCSR_H_
> > +
> > +#include 
> > +
> > +/*BCSR Utils functions*/
> > +void reset_p1021mds_micrel_phy(void);
> > +#endif  /* __BCSR_H_ */
> 
> Please avoid adding a new header file just for this single prototype.
This was copied from mpc8568mds/mpc8569mds. If it is not allowed anymore, I can 
remove it.

> Why do you need a separate one anyway? 
Because it is p1021mds board specific reset routine.
> 
> Why cannot you implement this
> in reset_phy() ?
reset_phy() is a one time reset and called after eth_initialize(). But
the board designer told me to reset phy for each UEC port before
initializing it, otherwise the phy can not work properly.

Haiying



___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 4/7] powerpc/p1021: Add P1021MDS board support

2010-08-16 Thread Wolfgang Denk 
Dear Haiying Wang,

In message <1281945949.24612.19.ca...@localhost.localdomain> you wrote:
> This patch supports P1021MDS NAND boot with the following features:
> * Boot from NAND flash with SRAM BOOT support.(No NOR flash on this board)
> * SPD DDR Initialization
> 
> Signed-off-by: Haiying Wang 
> Signed-off-by: Mohit Kumar 
> Signed-off-by: Yu.Liu 
> ---
>  MAKEALL   |1 +
>  Makefile  |4 +
>  board/freescale/p1021mds/Makefile |   38 ++
>  board/freescale/p1021mds/bcsr.c   |   22 +
>  board/freescale/p1021mds/bcsr.h   |   18 +
>  board/freescale/p1021mds/config.mk|   24 ++
>  board/freescale/p1021mds/ddr.c|  148 +++
>  board/freescale/p1021mds/law.c|   24 ++
>  board/freescale/p1021mds/p1021mds.c   |  122 ++
>  board/freescale/p1021mds/pci.c|   91 +
>  board/freescale/p1021mds/tlb.c|   72 
>  include/configs/P1021MDS.h|  536 
> +
>  nand_spl/board/freescale/p1021mds/Makefile|  117 ++
>  nand_spl/board/freescale/p1021mds/nand_boot.c |   59 +++
>  14 files changed, 1276 insertions(+), 0 deletions(-)

Entry to MAINTAINERS missing.

Additions of boards to Makefile are not allowed any more, please
configure in boards.cfg instead.

> +void reset_p1021mds_micrel_phy(void)
> +{
> + clrbits_8((u8 *)(CONFIG_SYS_BCSR_BASE + 11), BCSR11_ENET_MICRST);
> + setbits_8((u8 *)(CONFIG_SYS_BCSR_BASE + 11), BCSR11_ENET_MICRST);

Are you sure the reset pulse is long enough?

> diff --git a/board/freescale/p1021mds/bcsr.h b/board/freescale/p1021mds/bcsr.h
> new file mode 100644
> index 000..f3e47d4
> --- /dev/null
> +++ b/board/freescale/p1021mds/bcsr.h
> @@ -0,0 +1,18 @@
> +/*
> + * Copyright (C) 2010 Freescale Semiconductor, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + *
> + */
> +
> +#ifndef __BCSR_H_
> +#define __BCSR_H_
> +
> +#include 
> +
> +/*BCSR Utils functions*/
> +void reset_p1021mds_micrel_phy(void);
> +#endif  /* __BCSR_H_ */

Please avoid adding a new header file just for this single prototype.

Why do you need a separate one anyway? Why cannot you implement this
in reset_phy() ?


> diff --git a/nand_spl/board/freescale/p1021mds/Makefile 
> b/nand_spl/board/freescale/p1021mds/Makefile
> new file mode 100644
> index 000..2e88d72
> --- /dev/null
> +++ b/nand_spl/board/freescale/p1021mds/Makefile
...
> +ALL  = $(nandobj)u-boot-spl $(nandobj)u-boot-spl.bin 
> $(nandobj)u-boot-spl-16k.bin

Line too long. Please check and fix globally.


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: w...@denx.de
It is more rational to sacrifice one life than six.
-- Spock, "The Galileo Seven", stardate 2822.3
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot