Re: [U-Boot] [PATCH] Added support for the Guntermann & Drunck PowerPC 440 EP/GR ETX module.
Dear Dirk Eibach,
In message <[EMAIL PROTECTED]> you wrote:
>
> Subject: Added support for the Guntermann & Drunck PowerPC 440 EP/GR ETX
> module.
Please keep the subject well under 70 characters so it makes a good
title line for the commit message.
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -133,6 +133,7 @@ Jon Diekema <[EMAIL PROTECTED]>
> Dirk Eibach <[EMAIL PROTECTED]>
>
> neo PPC405EP
> + gdppc440etx PPC440EP/GR
Please keep lists sorted.
> diff --git a/board/gdsys/gdppc440etx/gdppc440etx.c
> b/board/gdsys/gdppc440etx/gdppc440etx.c
> new file mode 100644
> index 000..8903b61
> --- /dev/null
> +++ b/board/gdsys/gdppc440etx/gdppc440etx.c
> @@ -0,0 +1,494 @@
> +/*
> + * (C) Copyright 2008
> + * Dirk Eibach, Guntermann & Drunck GmbH, [EMAIL PROTECTED]
> + *
You claim exclusive copyright for this file. However, large parts of
this file are verbatim copies from elsewhere. For example, the PCI
code has extensively been "borrowed" from other boards. This is
perfectly legal, but please make sure to correctly attribute such
code. Otherwise the (legal) copying into (illegal) stealing.
> + /*clear tmrclk divisor */
> + *(unsigned char *)(CONFIG_SYS_BCSR_BASE | 0x04) = 0x00;
> +
> + /*enable ethernet */
> + *(unsigned char *)(CONFIG_SYS_BCSR_BASE | 0x08) = 0xf0;
> +
> + /*get rid of flash write protect */
> + *(unsigned char *)(CONFIG_SYS_BCSR_BASE | 0x07) = 0x00;
Please use I/O accessor functions/macros to write peripherals.
> +int misc_init_r (void)
> +{
> + uint pbcr;
> + int size_val = 0;
> +
> + /* Re-do sizing to get full correct info */
> + mtdcr(ebccfga, pb0cr);
> + pbcr = mfdcr(ebccfgd);
> + switch (gd->bd->bi_flashsize) {
> + case 1 << 20:
> + size_val = 0;
> + break;
> + case 2 << 20:
> + size_val = 1;
> + break;
> + case 4 << 20:
> + size_val = 2;
> + break;
> + case 8 << 20:
> + size_val = 3;
> + break;
> + case 16 << 20:
> + size_val = 4;
> + break;
> + case 32 << 20:
> + size_val = 5;
> + break;
> + case 64 << 20:
> + size_val = 6;
> + break;
> + case 128 << 20:
> + size_val = 7;
> + break;
> + }
How about providing a "default" entry?
And - wouldn't it be easier to drop the while switch and use
uint sz = (gd->bd->bi_flashsize >> 20);
for (size_val=0; (sz & 1) == 0; ++size_val)
sz >>= 1;
instead?
> + pbcr = (pbcr & 0x0001) | gd->bd->bi_flashstart | (size_val << 17);
...or combine the >> 20 above with the << 17 here ?
> + printf("Board: GDPPC440ETX - Guntermann & Drunck PPC440EP/GR
> ETX-module");
> +
> + rev = in_8((void *)(CONFIG_SYS_BCSR_BASE + 0));
> + val = in_8((void *)(CONFIG_SYS_BCSR_BASE + 5)) &
> CONFIG_SYS_BCSR5_PCI66EN;
Line length?
> +/*
> + * initdram -- doesn't use serial presence detect.
> + *
> + * Assumes:256 MB, ECC, non-registered
Do not hard-code sizes. Use aut-sizing, please.
> +void sdram_tr1_set(int ram_address, int* tr1_value)
> +{
> + int i;
> + int j, k;
> + volatile unsigned int* ram_pointer = (unsigned int*)ram_address;
> + int first_good = -1, last_bad = 0x1ff;
> +
> + unsigned long test[NUM_TRIES] = {
> + 0x, 0x, 0x, 0x,
> + 0x, 0x, 0x, 0x,
> + 0x, 0x, 0x, 0x,
> + 0x, 0x, 0x, 0x,
> + 0x, 0x, 0x, 0x,
> + 0x, 0x, 0x, 0x,
> + 0x, 0x, 0x, 0x,
> + 0x, 0x, 0x, 0x,
> + 0xA5A5A5A5, 0xA5A5A5A5, 0x5A5A5A5A, 0x5A5A5A5A,
> + 0xA5A5A5A5, 0xA5A5A5A5, 0x5A5A5A5A, 0x5A5A5A5A,
> + 0x5A5A5A5A, 0x5A5A5A5A, 0xA5A5A5A5, 0xA5A5A5A5,
> + 0x5A5A5A5A, 0x5A5A5A5A, 0xA5A5A5A5, 0xA5A5A5A5,
> + 0xAA55AA55, 0xAA55AA55, 0x55AA55AA, 0x55AA55AA,
> + 0xAA55AA55, 0xAA55AA55, 0x55AA55AA, 0x55AA55AA,
> + 0x55AA55AA, 0x55AA55AA, 0xAA55AA55, 0xAA55AA55,
> + 0x55AA55AA, 0x55AA55AA, 0xAA55AA55, 0xAA55AA55 };
> +
> + /* go through all possible SDRAM0_TR1[RDCT] values */
> + for (i=0; i<=0x1ff; i++) {
> + /* set the current value for TR1 */
> + mtsdram(mem_tr1, (0x80800800 | i));
> +
> + /* write values */
> + for (j=0; j + ram_pointer[j] = test[j];
> +
> + /* clear any cache at ram location */
> + __asm__("dcbf 0,%0": :"r" (&ram_pointer[j]));
> +
Re: [U-Boot] [PATCH] Added support for the Guntermann & Drunck PowerPC 440 EP/GR ETX module.
Hi Dirk, On Thursday 27 November 2008, Dirk Eibach wrote: > Signed-off-by: Dirk Eibach <[EMAIL PROTECTED]> Please find below some comments in addition to Wolfgangs comments: > diff --git a/board/gdsys/gdppc440etx/init.S > b/board/gdsys/gdppc440etx/init.S new file mode 100644 > index 000..f938236 > --- /dev/null > +++ b/board/gdsys/gdppc440etx/init.S > @@ -0,0 +1,112 @@ > +/* > +* > +* See file CREDITS for list of people who contributed to this > +* project. > +* > +* 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. > +* > +* This program is distributed in the hope that it will be useful, > +* but WITHOUT ANY WARRANTY; without even the implied warranty of > +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.See the > +* GNU General Public License for more details. > +* > +* You should have received a copy of the GNU General Public License > +* along with this program; if not, write to the Free Software > +* Foundation, Inc., 59 Temple Place, Suite 330, Boston, > +* MA 02111-1307 USA > +*/ > + > +#include > +#include > + > +/* General */ > +#define TLB_VALID 0x0200 > + > +/* Supported page sizes */ > + > +#define SZ_1K0x > +#define SZ_4K0x0010 ... Just include: #include and you get all those defines for free. Best regards, Stefan = DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: [EMAIL PROTECTED] = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Added support for the Guntermann & Drunck PowerPC 440 EP/GR ETX module.
Stefan Roese wrote: > Hi Dirk, > > On Thursday 27 November 2008, Dirk Eibach wrote: [snip] >> +#include >> +#include >> + >> +/* General */ >> +#define TLB_VALID 0x0200 >> + >> +/* Supported page sizes */ >> + >> +#define SZ_1K 0x >> +#define SZ_4K 0x0010 > ... > > Just include: > > #include > > and you get all those defines for free. > > Best regards, > Stefan Hi Stefan, I'll do you four letters fewer: #include ...since include/asm is symlinked to the active asm-. Best regards, gvb ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Added support for the Guntermann & Drunck PowerPC 440 EP/GR ETX module.
On Thursday 27 November 2008, Jerry Van Baren wrote: > > Just include: > > > > #include > > > > and you get all those defines for free. > > > > Best regards, > > Stefan > > Hi Stefan, > > I'll do you four letters fewer: Nice. ;) > #include > > ...since include/asm is symlinked to the active asm-. Of course. Good catch. Thanks, Stefan = DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: [EMAIL PROTECTED] = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Added support for the Guntermann & Drunck PowerPC 440 EP/GR ETX module.
Dear Jerry Van Baren, In message <[EMAIL PROTECTED]> you wrote: > > > #include ... > > I'll do you four letters fewer: > > #include > > ...since include/asm is symlinked to the active asm-. Thanks - well spotted. To summarize: asm-ppc is a four letters word :-) 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: [EMAIL PROTECTED] It is a good thing for an uneducated man to read books of quotations. - Sir Winston Churchill _My Early Life_ ch. 9 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Added support for the Guntermann & Drunck PowerPC 440 EP/GR ETX module.
Hello Wolfgang,
Thanks for the review. I'm busys cleaning up the code, but at one point
I need some help:
> >
> +/
> > +*
> > + * initdram -- doesn't use serial presence detect.
> > + *
> > + * Assumes:256 MB, ECC, non-registered
>
> Do not hard-code sizes. Use aut-sizing, please.
>
> > +void sdram_tr1_set(int ram_address, int* tr1_value) {
> > + int i;
> > + int j, k;
> > + volatile unsigned int* ram_pointer = (unsigned
> int*)ram_address;
> > + int first_good = -1, last_bad = 0x1ff;
> > +
> > + unsigned long test[NUM_TRIES] = {
> > + 0x, 0x, 0x, 0x,
> > + 0x, 0x, 0x, 0x,
> > + 0x, 0x, 0x, 0x,
> > + 0x, 0x, 0x, 0x,
> > + 0x, 0x, 0x, 0x,
> > + 0x, 0x, 0x, 0x,
> > + 0x, 0x, 0x, 0x,
> > + 0x, 0x, 0x, 0x,
> > + 0xA5A5A5A5, 0xA5A5A5A5, 0x5A5A5A5A, 0x5A5A5A5A,
> > + 0xA5A5A5A5, 0xA5A5A5A5, 0x5A5A5A5A, 0x5A5A5A5A,
> > + 0x5A5A5A5A, 0x5A5A5A5A, 0xA5A5A5A5, 0xA5A5A5A5,
> > + 0x5A5A5A5A, 0x5A5A5A5A, 0xA5A5A5A5, 0xA5A5A5A5,
> > + 0xAA55AA55, 0xAA55AA55, 0x55AA55AA, 0x55AA55AA,
> > + 0xAA55AA55, 0xAA55AA55, 0x55AA55AA, 0x55AA55AA,
> > + 0x55AA55AA, 0x55AA55AA, 0xAA55AA55, 0xAA55AA55,
> > + 0x55AA55AA, 0x55AA55AA, 0xAA55AA55, 0xAA55AA55 };
> > +
> > + /* go through all possible SDRAM0_TR1[RDCT] values */
> > + for (i=0; i<=0x1ff; i++) {
> > + /* set the current value for TR1 */
> > + mtsdram(mem_tr1, (0x80800800 | i));
> > +
> > + /* write values */
> > + for (j=0; j > + ram_pointer[j] = test[j];
> > +
> > + /* clear any cache at ram location */
> > + __asm__("dcbf 0,%0": :"r" (&ram_pointer[j]));
> > + }
> > +
> > + /* read values back */
> > + for (j=0; j > + for (k=0; k > + /* clear any cache at ram location */
> > + __asm__("dcbf 0,%0": :"r"
> (&ram_pointer[j]));
> > +
> > + if (ram_pointer[j] != test[j])
> > + break;
> > + }
> > +
> > + /* read error */
> > + if (k != NUM_READS) {
> > + break;
> > + }
> > + }
>
> Please do not implement yet another RAM test in U-Boot. We
> already have enough of them. If you really want to test your
> RAM, that is. For auto-sizing, you should use memsize() instead.
You are right, there is no RAM test necessary here. I want to use
auto-sizing with memsize(), but I did not find any refererences to that
in u-boot sources. Probaly I am missing something obvious. Can you show
me an example, where auto-sizing is used?
Cheers
Dirk
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Added support for the Guntermann & Drunck PowerPC 440 EP/GR ETX module.
Dear Dirk, In message <[EMAIL PROTECTED]> you wrote: > > Thanks for the review. I'm busys cleaning up the code, but at one point > I need some help: ... > You are right, there is no RAM test necessary here. I want to use > auto-sizing with memsize(), but I did not find any refererences to that > in u-boot sources. Probaly I am missing something obvious. Can you show > me an example, where auto-sizing is used? Sure. Many, actually: $ grep -Rl get_ram_size board/ board/LEOX/elpt860/elpt860.c board/RPXClassic/RPXClassic.c board/RPXlite/RPXlite.c board/RPXlite_dw/RPXlite_dw.c board/RRvision/RRvision.c board/a3000/a3000.c board/adder/adder.c board/atc/atc.c board/atmel/atngw100/atngw100.c board/atmel/atstk1000/atstk1000.c board/barco/barco.c board/bc3450/bc3450.c board/c2mon/c2mon.c board/canmb/canmb.c board/cm5200/cm5200.c board/cpu86/cpu86.c board/cpu87/cpu87.c board/cu824/cu824.c board/earthlcd/favr-32-ezkit/favr-32-ezkit.c board/ep88x/ep88x.c board/esd/cpci5200/cpci5200.c board/esd/mecp5200/mecp5200.c board/esd/pf5200/pf5200.c board/esteem192e/esteem192e.c board/etin/kvme080/kvme080.c board/etx094/etx094.c board/fads/fads.c board/genietv/genietv.c board/hermes/hermes.c board/hidden_dragon/hidden_dragon.c board/hmi1001/hmi1001.c board/icecube/icecube.c board/icu862/icu862.c board/ids8247/ids8247.c board/incaip/incaip.c board/inka4x0/inka4x0.c board/ip860/ip860.c board/iphase4539/iphase4539.c board/ispan/ispan.c board/ivm/ivm.c board/jupiter/jupiter.c board/lantec/lantec.c board/linkstation/linkstation.c board/lwmon/lwmon.c board/matrix_vision/mvbc_p/mvbc_p.c board/mcc200/mcc200.c board/keymile/mgcoge/mgcoge.c board/keymile/mgsuvd/mgsuvd.c board/mimc/mimc200/mimc200.c board/miromico/hammerhead/hammerhead.c board/motionpro/motionpro.c board/muas3001/muas3001.c board/mucmc52/mucmc52.c board/munices/munices.c board/musenki/musenki.c board/mvblue/mvblue.c board/nc650/nc650.c board/netphone/netphone.c board/netta/netta.c board/netta2/netta2.c board/nx823/nx823.c board/o2dnt/o2dnt.c board/oxc/oxc.c board/pm520/pm520.c board/pm826/pm826.c board/pn62/pn62.c board/r360mpi/r360mpi.c board/rbc823/rbc823.c board/rmu/rmu.c board/sandpoint/sandpoint.c board/sbc8240/sbc8240.c board/siemens/CCM/ccm.c board/siemens/IAD210/IAD210.c board/siemens/SCM/scm.c board/siemens/pcu_e/pcu_e.c board/sl8245/sl8245.c board/snmc/qs850/qs850.c board/snmc/qs860t/qs860t.c board/socrates/sdram.c board/sorcery/sorcery.c board/spc1920/spc1920.c board/spd8xx/spd8xx.c board/stxxtc/stxxtc.c board/tb0229/tb0229.c board/total5200/sdram.c board/tqc/tqm5200/tqm5200.c board/tqc/tqm8260/tqm8260.c board/tqc/tqm8272/tqm8272.c board/tqc/tqm834x/tqm834x.c board/tqc/tqm85xx/sdram.c board/tqc/tqm8xx/tqm8xx.c board/uc101/uc101.c board/utx8245/utx8245.c board/v38b/v38b.c board/xilinx/ppc440-generic/xilinx_ppc440_generic.c board/xilinx/ppc405-generic/xilinx_ppc405_generic.c 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: [EMAIL PROTECTED] The people of Gideon have always believed that life is sacred. That the love of life is the greatest gift ... We are incapable of destroying or interfering with the creation of that which we love so deeply -- life in every form from fetus to developed being. -- Hodin of Gideon, "The Mark of Gideon", stardate 5423.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Added support for the Guntermann & Drunck PowerPC 440 EP/GR ETX module.
On Thursday 27 November 2008 13:36, Dirk Eibach wrote:
> Signed-off-by: Dirk Eibach <[EMAIL PROTECTED]>
> +int board_early_init_f(void)
> +{
> + register uint reg;
> +
> + /*
> + * Setup the external bus controller/chip selects
> + *---*/
I am sure, but didn't we once said that these dashed lines are ugly and
multiline comments should look like this:
/*
* bla
* bla bla
*/
Matthias
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Added support for the Guntermann & Drunck PowerPC 440 EP/GR ETX module.
On Tue, 2008-12-02 at 09:17 +0100, Matthias Fuchs wrote:
> On Thursday 27 November 2008 13:36, Dirk Eibach wrote:
> > Signed-off-by: Dirk Eibach <[EMAIL PROTECTED]>
> > +int board_early_init_f(void)
> > +{
> > + register uint reg;
> > +
> > + /*
> > +* Setup the external bus controller/chip selects
> > +*---*/
>
> I am sure, but didn't we once said that these dashed lines are ugly and
> multiline comments should look like this:
>
> /*
>* bla
>* bla bla
>*/
Yes.
jdl
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
