Add first Netstal board HCU4

Hi

We at Netstal Maschinen (http://www.netstal.com/) will probably use the U-Boot in our next board, based on a PPC440EPx.

The reason we choose u-boot over a proprietary boot eprom are still the same as described in my earlier e-mail from February 2005
http://sourceforge.net/mailarchive/message.php?msg_id=10996256

Outside my activities at Netstal I had ported U-boot in 2005 to run on our current board (based on a ?PPC405GPr). I used it to get acquainted with Linux and Xenomai and one board is currently used by Xenomais buildbot. I would like to get this board into the U-Boot repository to have a solid base for our new board and to be able to test various features before getting hand onto our new board (somewhere in March/April).

As I am new to the U-Boot and Git, I am not sure, whether I got everything right in my first ?try to submit a patch. I did
a) ?git clone rsync://source.denx.net/git/u-boot.git u-boot.git
b) Added my files, testing them, etc
c) edited .git/config with my personal info
d) git-commit --all --signoff -m "Add first Netstal board HCU4"
e) gz'ed the resulting 0001-Add-first-Netstal-board-HCU4.txt to reduce the size from 52KB to 15 KB.
f) as between a) and c) passed various week I
g) git pull
As there were no conflicts, I rebuilded my target, tested again. Then I called
h) ./MAKEALL
Most target run okay, but I have errors like
Configuring for p3mx board...
p3mx.c:48:18: error: p3mx.h: No such file or directory p3mx.c:48:18: error: p3mx.h: No such file or directory
which I believe are unrelated to my changes.

I am unsatisfied that I had to add a flash.c to my board, but I have at the moment no idea how a clean solution should look like. But I think in the various flash.c there is a lot of duplicated code around.

As our board has ECC memory, I would like to submit in a second step a patch to get ECC working on ?PPC405.

Any suggestion on how to improve this patch are welcome.

Best regards

Niklaus Giger
--
As my first try from my workplace was considered spam I try it with my home e-mail.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-first-Netstal-board-HCU4.txt.gz
Type: application/x-gzip
Size: 15306 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20070209/3d3a05d8/attachment.bin

Hi Niklaus,

That's my fault. Fixed soon.

Yes, that has mostly historic reasons. So why do you have to use a board specific flash driver and not the common cfi driver?

So the current patch doesn't include this ECC support? Do you use the 4xx SPD driver (cpu/ppc/spd_sdram.c) or the no SPD driver (cpu/ppc/sdram.c)? Or even a board specif one?

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
=====================================================================

Am Samstag, 10. Februar 2007 08:59 schrieb Stefan Roese:

Because I started with a board that had a board specific flash.c, which worked. And a first attempt to use the common CFI failed, as I mixed too many changes. But I will try next week to follow you suggestion.

Yes it does not.

I did not want to mix in a single patch a new board and a new feature. And as it is the first time I work with git I not know how to easily work on two different patches at the same time. But I will try next week to come up with a patch for ECC.

I think I use cpu/ppc/spd_sdram.c as I call in my init_dram procedure ?
dram_size = spd_sdram(0);
Alternatively I first brought up my board using the known good values for the sdram registers in my procedure fixed_hcu4_sdram, which I left for debugging purposes #if !defined(CONFIG_SPD_EEPROM).

Thanks for your tips.

Best regards

Niklaus Giger

What flash devices do you use on your board?

Yes, you are using the common 4xx SPD driver then.

I'll try to take a deeper look at your patch on Monday.

You're welcome.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
=====================================================================

Hi

Attached you find a patch which enables ECC for our board. At the moment it is pretty PPC405GPr specific, as I have no other board around to test. I have also a Yosemite and a Sequoia, but they do not seem to have a correct SPD-info. I will submit a patch once I have our PPC440EPx board up and running.

For the startup I added a small hint that ECC is working:

I have a 8 bit boot-eprom AMD AM29LV040B
and 8,16 or 32 MB of CFI-Flash (one 16 bit-wide bank), e.g. Intel 28F028J3A

<..>
Best regards,
Niklaus

begin 666 ecc.patch M9&EF9BM+6=I="!A+V-P=2]P<&,T>'@O<W!D7W-D<F%M+F, at 8B]C<'4O<'!C M-'AX+W-P9%]S9')A;2YC"F]L9"!M;V1E(#$P,#8T-IN97<@;6]D92Q,#W M-34*:6YD97@@8S(T-#4V8BXN8V$Y,C)D-HM+2T@82]C<'4O<'!C-'AX+W-P M9%]S9')A;2YC"BLK*R!B+V-P=2]P<&,T>'@O<W!D7W-D<F%M+F,*0$@+30R M+#8@*S0R+#<@0$@(VEN8VQU9&4@/&-O;6UO;BYH/@H@(VEN8VQU9&4@/&%S M;2]P<F]C97-S;W(N:#X*("-I;F-L=61E(#QI,F,N:#X*("-I;F-L=61E(#QP M<&,T>'@N:#X**R-I;F-L=61E(#QA<VTO:6\N:#X*(H@(VEF9&5F($-/3D9) M1U]34$1?14504D]-"B*0$@+3DX+#8@*SDY+#@@0$@(V5L<V4*(",@9&5F M:6YE(%-01%]%4E(H>"D at 9&\@>R!P<FEN=&8H>"D[(')E='5R;B at P*3L@?2!W M:&EL92H,"D*("-E;F1I9 at H@"BMV;VED('!R;V=R86U?96-C("AV;VED("IB M86YK7V)A<V5?861D<BP@=6YS:6=N960@;&]N9PD@;G5M7V)Y=&5S+"!I;G0@ M8F%N:RD["BL*("-D969I;F4@<V1R86U?2%I?=&]?;G,H:&5R='HI("@Q,#P M,#P,#P+RAH97)T>BDI"B*("\J(&9U;F-T:6]N('!R;W1O='EP97,@*B\* M0$@+3,P-"PV("LS,#<L,3@0$@(V5N9&EF"B)"7-D<F%M,%]E8V-C9F<@ M/2P>&8@/#P at 4T1204TP7T5#0T-&1U]32$E&5#L*(D)96-C7V]N(#T@,3L* M(E](&5L<V4@>PHK(VEF(&1E9FEN960H0T].1DE'7T5#0RD@)B8 at 9&5F:6YE M9"A$14)51RD**PD)<')I;G1F*"(E<SH@;F\@14-#(&%S('-P9"Q,3H@)60@ M("V.BE9"Q-#H@)61<;B(L(%]?1E5.0U1)3TY?7RP**PD)("@("@(')E M861?<W!D*#$Q*2P@<F5A9%]S<&0H-BDL(')E861?<W!D*#$T*2D["BLC96YD M:68*(D)<V1R86TP7V5C8V-F9R](#["B)"65C8U]O;B](#["B)?0I M0"M-#(T+#<@*S0S,2PX($!("-E;F1I9 at H@"2J('!R;V=R86T at 86QL('1H M92!R96=I<W1E<G,N"B)("H at +2TM+2TM+2TM+2TM+2TM+2TM+2TM+2TM+2TM M+2TM+2TM+2TM+2TM+2TM+2TM+2TM+2TM+2TM+2TM+2TM+2TM+2TM+2HO"B* M+2-D969I;F4@;71S9')A;3H<F5G+"!D871A*2@;71D8W(H;65M8V9G82QR M96<I.VUT9&-R*&UE;6-F9V0L9&%T82D**R-D969I;F4@;71S9')A;3H<F5G M+"!D871A*2@("!M=&1C<BAM96UC9F=A+')E9RD[;71D8W(H;65M8V9G9"QD M871A*0HK(V1E9FEN92!M9G-D<F%M,"AR96<L(&1A=&$I("![(&UT9&-R*&UE M;6-F9V$L<F5G*3MD871A(#T@;69D8W(H;65M8V9G9"D[('T*(DO*B!D:7-A M8FQE(&UE;6-O;G1R;VQL97(@<V\@=7!D871E<R!W;W)K("HO"B);71S9')A M;3H(&UE;5]M8V]P=#$L(#@*3L*(I0"M-#0W+#$P("LT-34L,3@0$@ M(V5N9&EF"B)+RH at 4T1204T@:&%V92!A('!O=V5R(&]N(&1E;&%Y+D@-3P M(&UI8W)O('-H;W5L9"!D;RJ+PH@"75D96QA>2 at U,#I.PH@"7-D<F%M,%]C M9F<@/2!31%)!33!?0T9'7T1#12!\(%-$4D%-,%]#1D=?0E)01B at Q*2!\(%-$ M4D%-,%]#1D=?14-#1$0@?"!31%)!33!?0T9'7T5-1%5,4CL*+0EI9BH96-C M7V]N*0HM"0ES9')A;3!?8V9G('P](%-$4D%-,%]#1D=?345-0TA+.PH@"6UT M<V1R86TP*&UE;5]M8V]P=#$L('-D<F%M,%]C9F<I.PHM"BLC:69D968@0T]. M1DE'7T5#0PHK"6EF("AE8V-?;VXI('!R;V=R86U?96-C*#L('1O=&%L7W-I M>F4L(#I.PHK(V5N9&EF"B)<F5T=7)N("AT;W1A;%]S:7IE*3L*('T*(I M0"M-CDV+#@@*S<P-"PV($!('9O:60@<')O9W)A;5]T<C@*'5N<VEG;F5D M(&QO;F<J(&1I;6U?<&\*(H@=F]I9"!P<F]G<F%M7W1R,2H=F]I9"D["B* M+79O:60@<')O9W)A;5]E8V,@*'5N<VEG;F5D(&QO;F<)(&YU;5]B>71E<RD[ M"BT*('5N<VEG;F5D"B!L;VYG("!P<F]G<F%M7V)X8W(H=6YS:6=N960@;&]N M9RH at 9&EM;5]P;W!U;&%T960L"B)"2@('5N<VEG;F5D(&-H87(J(&EI8S!? M9&EM;5]A9&1R+I0"M,3<X-2PT-RK,3<Y,2PW-B!0"C96YD:68*(ER M971U<FXH8F%N:U]B87-E7V%D9'(I.PH@?0H@"BUV;VED('!R;V=R86U?96-C M("AU;G-I9VYE9"!L;VYG"2!N=6U?8GET97,I"BLC96YD:68 at +RH@0T].1DE' M7S0T,"J+PHK"BLC9&5F:6YE(%-$4D%-7T5#0T-&1U]#13)"3!X,#X,#P M,#)+RH at 14-#($-O<G)E8W1I;VX at 16YA8FQE(&9O<B!"86YK(#)*B\**R-D M969I;F4 at 4T1204U?14-#0T9'7T-%,0D),'@P,#0P,#P,DO*B!%0T, at 0V]R M<F5C=&EO;B!%;F%B;&4@9F]R($)A;FL@,0DJ+PHK(V1E9FEN92!31%)!35]% M0T-#1D=?0T4R"0DP>#P,CP,#P"2\J($5#0R!#;W)R96-T:6]N($5N86)L M92!F;W(@0F%N:RR"2HO"BLC9&5F:6YE(%-$4D%-7T5#0T-&1U]#13,)"3!X M,#Q,#P,#)+RH at 14-#($-O<G)E8W1I;VX at 16YA8FQE(&9O<B!"86YK(#,) M*B\**PHK(V1E9FEN92!31%)!35]%0T-%4U)?15)23U)?34%32R@("@("@ M(#!X1D9&,$8P,#@("@("O*B!!;&P@<&]S<VEB;&4@14-#(&5R<F]R<RJ M+PHK(V1E9FEN92!%0T-?5$535%]604Q512P>&%F9F5A9F9E"BL**R\J(HK M("H at 4')E<&%R92!F;W(@14-#(&]P97)A=&EO;@HK("H at 4W1E<"Q.B!%;F%B M;&4 at 14-#(&=E;F5R871I;VX at 8G5T(&YO="!C:&5C:W,**RJ(%-T97@,CH@ M1FEL;"!A;&P@;65M;W)Y"BL@*B!3=&5P(#,Z($5N86)L92!%0T, at 9V5N97)A M=&EO;B!A;F0 at 8VAE8VMS"BL@*B!/;FQY('!R;V=R86UM960 at 9F]R(&%N9"!T M97-T960@;VX@82!04$,T,#5'4'(@8F]A<F0@=7-I;F<@;VYL>2!B86YK(#@ M86YD(#,R(&)I="!W:61E("$A(0HK("HO"BMV;VED('!R;V=R86U?96-C("AV M;VED("IB86YK7V)A<V5?861D<BP@=6YS:6=N960@;&]N9R!N=6U?8GET97,L M(&EN="!B86YK*0H@>PHM"75N<VEG;F5D(&QO;F<@8F%N:U]B87-E7V%D9'([ M"B)=6YS:6=N960@;&]N9R!C=7)R96YT7V%D9')E<W,["B)=6YS:6=N960@ M;&]N9R!E;F1?861D<F5S<SL*(EU;G-I9VYE9"!L;VYG(&%D9')E<W-?:6YC M<F5M96YT.PH@"75N<VEG;F5D(&QO;F<@8V9G,#L**PEI9BAB86YK("$](#I M('L**PD)<')I;G1F*")<;B5S.B!O;FQY(&)A;FL@,"!S=7!P;W)T960B+"@ M7U]&54Y#5$E/3E]?*3L**PE]"B*(DO*@H@"2J(&=E="!-96UO<GD at 0V]N M=')O;&QE<B!/<'1I;VYS(#@9&%T80H@"2J+PHM"6UF<V1R86TH;65M7V-F M9SL(&-F9SI.PHM"BL);69S9')A;3H;65M7VUC;W!T,2P at 8V9G,"D["BL) M"BL)8V9G,"F/2!^4T1204TP7T-&1U]%34153%(@)B!^4T1204TP7T-&1U]- M14U#2$L["BLC:69D968@1$5"54<**PEP<FEN=&8H(B5S.B!L96YG=&@@,'@E M>"!B>71E<R!B86YK7V)A<V5?861D<BE<%QN(BP@(%]?1E5.0U1)3TY?7RP@ M(&YU;5]B>71E<RP at 8F%N:U]B87-E7V%D9'(I.PHK"7!R:6YT9B@B)7,Z(&-F M9S@9&ES86)L92!C:&5C:VEN9RM/BP>"4P.'A<;B(L("!?7T953D-424]. M7U\L("!C9F<P*3L**R-E;F1I9 at H@"2\J"B)("H@<F5S970@=&AE(&)A;FM? M8F%S92!A9&1R97-S"B)("HO"BT)8F%N:U]B87-E7V%D9'(@/2!#1D=?4T12 M04U?0D%313L**PEM='-D<F%M,"AM96U?96-C8V8L("P*3L@("@("@("@ M+RH at 9&ES86)L92!C;W)R96-T:6]N("HO"BL);71S9')A;3H;65M7V5C8V5R M<BP at 4T1204U?14-#15-27T524D]27TU!4TLI.RO*B!#;&5A<B!A;&P at 97)R M;W)S("HO"BL);71S9')A;3H;65M7VUC;W!T,2P at 8V9G,"D["B*+0EI9BH M*&-F9S@)B!31%)!35]#1D<P7TU#2$M?34%32RD@(3T@4T1204U?0T9',%]- M0TA+7TY/3BD@>PHM"0EM='-D<F%M*&UE;5]C9F<P+"H8V9G,"F('Y31%)! M35]#1D<P7TU#2$M?34%32RD@?HM"0D)4T1204U?0T9',%]-0TA+7T=%3BD[ M"BL)861D<F5S<U]I;F-R96UE;G0@/2T.PHK"6-U<G)E;G1?861D<F5S<R] M("AU;G-I9VYE9"!L;VYG*2AB86YK7V)A<V5?861D<BD["BL)96YD7V%D9')E M<W,@/2H=6YS:6=N960@;&]N9RDH8F%N:U]B87-E7V%D9'(I("L@;G5M7V)Y M=&5S.PH@"BT)"6EF("@H8V9G,"F(%-$4D%-7T-&1S!?1$U71%]-05-+*2`] M/2!31%)!35]#1D<P7T1-5T1?,S(I('L*+0D)"6%D9')E<W-?:6YC<F5M96YT M(#T at -#L*+0D)?2!E;'-E('L*+0D)"6%D9')E<W-?:6YC<F5M96YT(#T@.#L* M+0D)?0HK"7=H:6QE("AC=7)R96YT7V%D9')E<W,@/"!E;F1?861D<F5S<RD@ M>PHK"0DJ*"AU;G-I9VYE9"!L;VYG*BEC=7)R96YT7V%D9')E<W,I(#T@,#L* M*PD)8W5R<F5N=%]A9&1R97-S("L](&%D9')E<W-?:6YC<F5M96YT.PHK"7T* M(HM"0EC=7)R96YT7V%D9')E<W,@/2H=6YS:6=N960@;&]N9RDH8F%N:U]B M87-E7V%D9'(I.PHM"0EE;F1?861D<F5S<R]("AU;G-I9VYE9"!L;VYG*2AB M86YK7V)A<V5?861D<BD@*R!N=6U?8GET97,["BL);71S9')A;3H;65M7V5C M8V5R<BP at 4T1204U?14-#15-27T524D]27TU!4TLI.RO*B!#;&5A<B!A;&P@ M97)R;W)S("HO"B*+0D)=VAI;&4@*&-U<G)E;G1?861D<F5S<R\(&5N9%]A M9&1R97-S*2!["BT)"0DJ*"AU;G-I9VYE9"!L;VYG*BEC=7)R96YT7V%D9')E M<W,I(#T@,'@P,#P,#P,#L*+0D)"6-U<G)E;G1?861D<F5S<RK/2!A9&1R M97-S7VEN8W)E;65N=#L*+0D)?0HK(VEF9&5F($1%0E5'"BL)<')I;G1F*"(E M<SH at 8V9G,"!E;F%B;&4 at 8VAE8VMI;F=<;B(L("!?7T953D-424].7U\I.PHK M(V5N9&EF"BL);71S9')A;3H;65M7V5C8V-F+"!31%)!35]%0T-#1D=?0T4P M*3L@("@("@("O*B!E;F%B;&4 at 8V]R<F5C=&EO;BJ+PHK"7!R:6YT9B at B M14-#("(I.PH@"BT)"6UT<V1R86TH;65M7V-F9SL("AC9F<P("8@?E-$4D%- M7T-&1S!?34-(2U]-05-+*2!\"BT)"0E31%)!35]#1D<P7TU#2$M?0TA+*3L* M*R-I9F1E9B!$14)51PHK"7L at +RH@02!S;6%L;"!S86YI='D at 8VAE8VL@*B\* M*PD)=6YS:6=N960@;&]N9RJ8VAE8VL["BL)"6-H96-K/2H=6YS:6=N960@ M;&]N9RJ*6)A;FM?8F%S95]A9&1R.PHK"0DJ8VAE8VL]14-#7U1%4U1?5D%, M544["BL)"6EF*"IC:&5C:RA/2!%0T-?5$535%]604Q512D**PD)"7!R:6YT M9B@B)7,Z(&-H96-K:6YG(&%T("5P(&ES(#!X)7@@9F%I;&5D7&XB+"@7U]& M54Y#5$E/3E]?+"!C:&5C:RP@*F-H96-K*3L*(E]"BLC96YD:68)"B!]"B* M+2-E;F1I9BO*B!#3TY&24=?-#0P("HO"BT*("-E;F1I9BO*B!#3TY&24=? ..4U!$7T5%4%)/32J+PH`
`
end

Hi Niklaus,

Sorry, can't decode the attachment. Could you please send it inline? Thanks.

The 440EP (Yosemite) has a DDR controller and the 440EPx (Sequoia) has a total different DDR2 controller. And both boards you mentioned are not equipped with DIMM modules and therefore don't have an SPD EEPROM.

How far are with your 440EPx porting?

Autsch! I really don't like such braindead designs (8bit _and_ 16bit flash on one EBC). Makes software really complicated. Please don't do this on your 440EPx design. ;-)

BTW: Sorry, but I the review of your patch will take a little longer, since we are on the Embedded World fair in Nuerberg this week.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
=====================================================================

Here it is:

old mode 100644
new mode 100755
index c24456b..ca922d4
@@ -42,6 +42,7 @@ #include <common.h>
 #include <asm/processor.h>
 #include <i2c.h>
 #include <ppc4xx.h>
+#include <asm/io.h>
 
 #ifdef CONFIG_SPD_EEPROM
 

@@ -98,6 +99,8 @@ #else
 # define SPD_ERR(x) do { printf(x); return(0); } while (0)
 #endif
 
+void program_ecc (void *bank_base_addr, unsigned long	 num_bytes, int bank);
+
 #define sdram_HZ_to_ns(hertz) (1000000000/(hertz))
 
 /* function prototypes */

@@ -304,6 +307,10 @@ #endif
 		sdram0_ecccfg = 0xf << SDRAM0_ECCCFG_SHIFT;
 		ecc_on = 1;
 	} else {
+#if defined(CONFIG_ECC) && defined(DEBUG)
+		printf("%s: no ECC as spd 11: %d   6: %d 14: %d\n", __FUNCTION__,
+		       read_spd(11), read_spd(6), read_spd(14));
+#endif
 		sdram0_ecccfg = 0;
 		ecc_on = 0;
 	}

@@ -424,7 +431,8 @@ #endif
 	 * program all the registers.
 	 * -------------------------------------------------------------------*/
 
-#define mtsdram0(reg, data)  mtdcr(memcfga,reg);mtdcr(memcfgd,data)
+#define mtsdram0(reg, data)    mtdcr(memcfga,reg);mtdcr(memcfgd,data)
+#define mfsdram0(reg, data)  { mtdcr(memcfga,reg);data = mfdcr(memcfgd); }
 	/* disable memcontroller so updates work */
 	mtsdram0( mem_mcopt1, 0 );
 

@@ -447,10 +455,10 @@ #endif
 	/* SDRAM have a power on delay,	 500 micro should do */
 	udelay(500);
 	sdram0_cfg = SDRAM0_CFG_DCE | SDRAM0_CFG_BRPF(1) | SDRAM0_CFG_ECCDD |

SDRAM0_CFG_EMDULR;
- if (ecc_on)
- sdram0_cfg |= SDRAM0_CFG_MEMCHK;
mtsdram0(mem_mcopt1, sdram0_cfg);
-
+#ifdef CONFIG_ECC
+ if (ecc_on) program_ecc(0, total_size, 0);
+#endif
return (total_size);
}

@@ -696,8 +704,6 @@ void program_tr0 (unsigned long* dimm_po

void program_tr1 (void);

-void program_ecc (unsigned long num_bytes);
-
unsigned
long program_bxcr(unsigned long* dimm_populated,
unsigned char* iic0_dimm_addr,
@@ -1785,47 +1791,76 @@ #endif
return(bank_base_addr);
}

-void program_ecc (unsigned long num_bytes)
+#endif /* CONFIG_440 */
+
+#define SDRAM_ECCCFG_CE0 0x00800000 /* ECC Correction Enable for Bank 0 */
+#define SDRAM_ECCCFG_CE1 0x00400000 /* ECC Correction Enable for Bank 1 */
+#define SDRAM_ECCCFG_CE2 0x00200000 /* ECC Correction Enable for Bank 2 */
+#define SDRAM_ECCCFG_CE3 0x00100000 /* ECC Correction Enable for Bank 3 */
+
+#define SDRAM_ECCESR_ERROR_MASK 0xFFF0F000 /* All possible ECC
errors */
+#define ECC_TEST_VALUE 0xaffeaffe
+
+/*
+ * Prepare for ECC operation
+ * Step 1: Enable ECC generation but not checks
+ * Step 2: Fill all memory
+ * Step 3: Enable ECC generation and checks
+ * Only programmed for and tested on a PPC405GPr board using only bank 0 and 32 bit wide !!!
+ */
+void program_ecc (void *bank_base_addr, unsigned long num_bytes, int bank)
{
- unsigned long bank_base_addr;
unsigned long current_address;
unsigned long end_address;
unsigned long address_increment;
unsigned long cfg0;
+ if(bank != 0) {
+ printf("\n%s: only bank 0 supported", __FUNCTION__);
+ }

/*
* get Memory Controller Options 0 data
*/
- mfsdram(mem_cfg0, cfg0);
-
+ mfsdram0(mem_mcopt1, cfg0);
+
+ cfg0 &= ~SDRAM0_CFG_EMDULR & ~SDRAM0_CFG_MEMCHK;
+#ifdef DEBUG
+ printf("%s: length 0x%x bytes bank_base_addr %p\n", __FUNCTION__, num_bytes,
bank_base_addr);
+ printf("%s: cfg0 disable checking -> 0x%08x\n", __FUNCTION__, cfg0);
+#endif
/*
* reset the bank_base address
*/
- bank_base_addr = CFG_SDRAM_BASE;
+ mtsdram0(mem_ecccf, 0); /* disable correction */
+ mtsdram0(mem_eccerr, SDRAM_ECCESR_ERROR_MASK); /* Clear all errors */
+ mtsdram0(mem_mcopt1, cfg0);

- if ((cfg0 & SDRAM_CFG0_MCHK_MASK) != SDRAM_CFG0_MCHK_NON) {
- mtsdram(mem_cfg0, (cfg0 & ~SDRAM_CFG0_MCHK_MASK) |
- SDRAM_CFG0_MCHK_GEN);
+ address_increment = 4;
+ current_address = (unsigned long)(bank_base_addr);
+ end_address = (unsigned long)(bank_base_addr) + num_bytes;

- if ((cfg0 & SDRAM_CFG0_DMWD_MASK) == SDRAM_CFG0_DMWD_32) {
- address_increment = 4;
- } else {
- address_increment = 8;
- }
+ while (current_address < end_address) {
+ *((unsigned long*)current_address) = 0;
+ current_address += address_increment;
+ }

- current_address = (unsigned long)(bank_base_addr);
- end_address = (unsigned long)(bank_base_addr) + num_bytes;
+ mtsdram0(mem_eccerr, SDRAM_ECCESR_ERROR_MASK); /* Clear all errors */

- while (current_address < end_address) {
- *((unsigned long*)current_address) = 0x00000000;
- current_address += address_increment;
- }
+#ifdef DEBUG
+ printf("%s: cfg0 enable checking\n", __FUNCTION__);
+#endif
+ mtsdram0(mem_ecccf, SDRAM_ECCCFG_CE0); /* enable correction */
+ printf("ECC ");

- mtsdram(mem_cfg0, (cfg0 & ~SDRAM_CFG0_MCHK_MASK) |
- SDRAM_CFG0_MCHK_CHK);
+#ifdef DEBUG
+ { /* A small sanity check */
+ unsigned long *check;
+ check= (unsigned long *)bank_base_addr;
+ *check=ECC_TEST_VALUE;
+ if(*check != ECC_TEST_VALUE)
+ printf("%s: checking@%p is 0x%x failed\n", __FUNCTION__, check,
*check);
}
+#endif
}

-#endif /* CONFIG_440 */
-
#endif /* CONFIG_SPD_EEPROM */

<..>

It is basically a copy of the sequoia minus NAND and NOR chips, as we will only have a Boot-Flash which will fetch everything into RAM from our human interface board. Layout is under way, first HW will come late in March or sometimes in April.

As stated above we only will have 8 bit boot-eprom AMD AM29LV040B. And I do not care if our old board has no support for the CFI.

No problem. I can wait.

Best regards

Niklaus

Hi Niklaus,

Line wrapped. Please make sure next time, that your mail client doesn't wrap the lines.

#if defined(CONFIG_ECC)
		debug("%s: no ECC as spd 11: %d   6: %d 14: %d\n", __FUNCTION__,
		     read_spd(11), read_spd(6), read_spd(14));

Coding style.

I will review the next version more thoroughly. And faster, I promise.

Sorry again for the delay.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
=====================================================================

Hi Stefan

Here my reworked patch. Tried a follow all of your comments.

 cpu/ppc4xx/40x_spd_sdram.c |   86
++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 83 insertions(+), 3 deletions(-)

index 19c4f76..f1e9b38 100644
@@ -104,6 +104,7 @@
 
 /* function prototypes */
 int spd_read(uint addr);
+void program_ecc (void *bank_base_addr, unsigned long num_bytes, int bank);
 
 
 /*

@@ -306,6 +307,10 @@ long int spd_sdram(int(read_spd)(uint addr))
                sdram0_ecccfg = 0xf << SDRAM0_ECCCFG_SHIFT;
                ecc_on = 1;
        } else {
+#if defined(CONFIG_ECC)
+               debug("%s: no ECC as spd 11: %d   6: %d 14: %d\n", __FUNCTION__,
+                      read_spd(11), read_spd(6), read_spd(14));
+#endif
                sdram0_ecccfg = 0;
                ecc_on = 0;
        }

@@ -426,7 +431,9 @@ long int spd_sdram(int(read_spd)(uint addr))
         * program all the registers.
         * -------------------------------------------------------------------*/
 
-#define mtsdram0(reg, data)  mtdcr(memcfga,reg);mtdcr(memcfgd,data)
+#define mfsdram0(reg, data)  { mtdcr(memcfga,reg);data = mfdcr(memcfgd); }
+#define mtsdram0(reg, data)    mtdcr(memcfga,reg);mtdcr(memcfgd,data)
+
        /* disable memcontroller so updates work */
        mtsdram0( mem_mcopt1, 0 );
 

@@ -449,9 +456,11 @@ long int spd_sdram(int(read_spd)(uint addr))
        /* SDRAM have a power on delay,  500 micro should do */
        udelay(500);
        sdram0_cfg = SDRAM0_CFG_DCE | SDRAM0_CFG_BRPF(1) | SDRAM0_CFG_ECCDD |

SDRAM0_CFG_EMDULR;
- if (ecc_on)

-               sdram0_cfg |= SDRAM0_CFG_MEMCHK;
        mtsdram0(mem_mcopt1, sdram0_cfg);

+#ifdef CONFIG_ECC
+ if (ecc_on)
+ program_ecc(0, total_size, 0);
+#endif

        return (total_size);
 }
@@ -466,4 +475,75 @@ int spd_read(uint addr)
                return 0;

}

+#define SDRAM_ECCCFG_CE0 0x00800000 /* ECC Correction Enable for Bank 0 */ +#define SDRAM_ECCCFG_CE1 0x00400000 /* ECC Correction Enable for Bank 1 */ +#define SDRAM_ECCCFG_CE2 0x00200000 /* ECC Correction Enable for Bank 2 */ +#define SDRAM_ECCCFG_CE3 0x00100000 /* ECC Correction Enable for Bank 3 */ +
+#define SDRAM_ECCESR_ERROR_MASK 0xFFF0F000 /* All possible ECC errors */ +#define ECC_TEST_VALUE 0xaffeaffe
+
+/*
+ * Prepare for ECC operation
+ * Step 1: Enable ECC generation but not checks
+ * Step 2: Fill all memory
+ * Step 3: Enable ECC generation and checks
+ * Only programmed for and tested on a PPC405GPr board using:
+ * bank 0 and 32 bit wide !!!
+ */
+void program_ecc (void *bank_base_addr, unsigned long num_bytes, int bank) +{

+       unsigned long current_address;
+       unsigned long end_address;
+       unsigned long address_increment;
+       unsigned long cfg0;
+       if (bank != 0)
+       {

+ printf("\n%s: only bank 0 supported", __FUNCTION__);
+ return;
+ }
+
+ /*
+ * get Memory Controller Options 0 data
+ */
+ mfsdram0(mem_mcopt1, cfg0);
+
+ cfg0 &= ~SDRAM0_CFG_EMDULR & ~SDRAM0_CFG_MEMCHK;
+ debug("%s: length 0x%x bytes bank_base_addr %p\n", __FUNCTION__,
+ num_bytes, bank_base_addr);
+ debug("%s: cfg0 disable checking -> 0x%08x\n", __FUNCTION__, cfg0);
+ /*
+ * reset the bank_base address
+ */

+       mtsdram0(mem_ecccf,  0); /* disable correction */
+       mtsdram0(mem_eccerr, SDRAM_ECCESR_ERROR_MASK); /* Clear all errors */
+       mtsdram0(mem_mcopt1, cfg0);
+
+       address_increment = 4;
+       current_address = (unsigned long)(bank_base_addr);
+       end_address = (unsigned long)(bank_base_addr) + num_bytes;

+
+ while (current_address < end_address) {
+ *((unsigned long*)current_address) = 0;
+ current_address += address_increment;
+ }
+
+ mtsdram0(mem_eccerr, SDRAM_ECCESR_ERROR_MASK); /* Clear all errors */
+

+       debug("%s: cfg0 enable checking\n",  __FUNCTION__);
+       mtsdram0(mem_ecccf, SDRAM_ECCCFG_CE0); /* enable correction */
+       printf("ECC ");
+
+#ifdef DEBUG
+       { /* A small sanity check */
+               unsigned long *check;
+               check= (unsigned long *)bank_base_addr;
+               *check=ECC_TEST_VALUE;
+               if (*check != ECC_TEST_VALUE)

+ debug("%s: checking@%p is 0x%x failed\n",
+ __FUNCTION__, check, *check);
+ }
+#endif
+}
+
#endif /* CONFIG_SPD_EEPROM */

Best regards

Niklaus Giger

Hi Niklaus,

Thanks. Just wanted to let you know that I have seen your patches and will review them directly after the Easter vacation.

Viele Gr??e,
Stefan

=====================================================================
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
=====================================================================

Dear Niklaus,

This is bad style. Any such multi-statement macro definition should be encapsulated in a "do { ... } while (0)" frame.

DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
All your people must learn before you can reach for the stars.
	-- Kirk, "The Gamesters of Triskelion", stardate 3259.2

Dear Niklaus,

There are some coding style violations: indentation not by TAB.

Please write comments in English language, thanks.

include/configs/hcu4.h is formatted in a way that makes it mostly unreadable. You may want to clean this up a bit.

You use a private flash driver for something that looks as if it was CFI compatible - why don't you use the CFI driver?

Best regards,

Wolfgang Denk

DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Where shall I begin, please your Majesty?" he asked. "Begin  at  the
beginning,"  the  King said, gravely, "and go on till you come to the
end: then stop."    - Alice's Adventures in Wonderland, Lewis Carroll

This has been discussed before (I asked the same question). IIRC the board used an 8bit wide non CFI-compatible FLASH and an 16bit wide CFI compatible FLASH device. With the current CFI implementation this setup is not supported so a "custom" driver has to be used.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
=====================================================================

Hi Niklaus,

please find some comments below:

	cfg0 &= ~(SDRAM0_CFG_EMDULR | SDRAM0_CFG_MEMCHK);

Hi Niklaus,

sorry for the delay. Could you please resubmit the HCU4 patch against the current top-of-git repository? I promise to review it now quickly.

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
=====================================================================

Hi Stefan

Here it is. bzipped because of the size limitation. I reworked it slightly and tried harder to follow the Linux kernel coding style.

I used the following commands for my workflow (Could not find a clear entry in the WiKi for a simple example tailored to the u-boot).

$ git clone http://source.denx.net/git/u-boot.git u-boot.git
$ cd u-boot.git

Then I added the following lines to .git/config

<added/edited some files>

$ git add <for each new directory or file>
Used "git status" to check whether I added all new files correcty.
Used "git diff origin --check" to look for trailing/superfluous
whitespaces.
Used "git diff origin -p --summary origin" to preview my changes.
Once I was satisfied, I preceeded with
$ git-commit --all --signoff -m "Add first Netstal board HCU4: Second
try"
$ git-format-patch origin
$ mv 0001-Add-first-Netstal-board-HCU4-Second-try.patch \
  Add-first-Netstal-board-HCU4-Second-try.patch
$ gzip Add-first-Netstal-board-HCU4-Second-try.patch

Is this a good workflow?

the

I will be absent from office the next two weeks, but will have a look at your comments sooner.

Best regards

Niklaus Giger
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Add-first-Netstal-board-HCU4-Second-try.patch.gz
Type: application/x-gzip
Size: 15319 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20070406/9494f8bf/attachment.bin 

Hi Niklaus,

Thanks. That's good. But unfortunately you had to gzip it because of the size limitation. This would not have been necessary if you had split your patch up into 2 or 3 mails. I would recommend something like:

[PATCH 1/3]: ppc4xx: Add Netstal common board directory [PATCH 2/3]: ppc4xx: Add Netstal HCU4 board support [PATCH 3/3]: ppc4xx: Add Netstal HCU4 board config file

In-line patches make my reviewing easier. Thanks.

Please add a little more descriptive commit message: Like which CPU is used.

I'm still using cogito, so I can't really comment on the core git usage.

Please clean up the issues Wolfgang addressed and resubmit. Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
=====================================================================