I agree. I would not have written that way had it been from cratch. tried to follow the code that was already there.

buttried to follow the alre

On 7/3/07, Grant Likely grant.likely@secretlab.ca wrote:

On 7/3/07, eran.liberty@gmail.com eran.liberty@gmail.com wrote:

...

Adds generic implementation of Altera Stratix II fpga burn both in: fast passive parallel, and fast passive parallel with security, formats.

Signed-off-by: Eran Liberty eran.liberty@gmail.com

Mostly looks okay; except for how it's hooked into the fpga command. Comments below.

g.

...

Index: common/altera.c

--- common/altera.c (.../tags/trunk/20070620_2_merge_to_exsw6000) (revision 69) +++ common/altera.c (.../branches/exsw6000) (revision 69) @@ -1,4 +1,7 @@ /*

• • (C) Copyright 2007
• • Eran Liberty, Extricom , eran.liberty@gmail.com
• • (C) Copyright 2003
  • Steven Scholz, imc Measurement & Control, steven.scholz@imc-berlin.de

@@ -30,6 +33,7 @@ */ #include <common.h> #include <ACEX1K.h> +#include <stratixII.h>

/* Define FPGA_DEBUG to get debug printf's */ /* #define FPGA_DEBUG */ @@ -43,7 +47,7 @@ #if (CONFIG_FPGA & CFG_FPGA_ALTERA)

/* Local Static Functions */ -static int altera_validate (Altera_desc * desc, char *fn); +static int altera_validate (Altera_desc * desc,const char *fn);

/* ------------------------------------------------------------------------- */ int altera_load( Altera_desc *desc, void *buf, size_t bsize ) @@ -69,6 +73,16 @@ __FUNCTION__); #endif break;

•           case Altera_StratixII:
  

+#if (CONFIG_FPGA & CFG_STRATIX_II)

•                   PRINTF ("%s: Launching the Stratix II Loader...\n",
  

•                           __FUNCTION__);
  

•                   ret_val = StratixII_load (desc, buf, bsize);
  

+#else

•                   printf ("%s: No support for Stratix II devices.\n",
  

•                           __FUNCTION__);
  

+#endif

•                   break;
  

Yuck; I wouldn't do this. Dump the else and put the 'case' inside the conditional (and fix Altera_ACEX1K while you're at it). Otherwise you're adding size to all boards which use altera.c for no real benefit.

Agree! will do.

...

            default:
                    printf ("%s: Unsupported family type, %d\n",

@@ -97,6 +111,16 @@ __FUNCTION__); #endif break;

•           case Altera_StratixII:
  

+#if (CONFIG_FPGA & CFG_STRATIX_II)

•                   PRINTF ("%s: Launching the Stratix II Reader...\n",
  

•                           __FUNCTION__);
  

•                   ret_val = StratixII_dump (desc, buf, bsize);
  

+#else

•                   printf ("%s: No support for Stratix II devices.\n",
  

•                           __FUNCTION__);
  

+#endif

•                   break;
  
            default:
                    printf ("%s: Unsupported family type, %d\n",
  

@@ -117,6 +141,9 @@ case Altera_ACEX1K: printf ("ACEX1K\n"); break;

•           case Altera_StratixII:
  

•                   printf ("Stratix II\n");
  

•                   break;
  

I think all this stuff should really be table driven (instead of using switch statements). You should create a structure containing the relevant strings & functions, and statically populate it based on what support is configured in.

I do want to convert it to table driven myself. Or in other words, though I basically agree I prefer to ignore it.

...

                    /* Add new family types here */
            case Altera_CYC2:
                    printf ("CYCLON II\n");

@@ -142,7 +169,14 @@ case altera_jtag_mode: /* Not used */ printf ("JTAG Mode\n"); break;

•           case fast_passive_parallel:
  

•                   printf ("Fast Passive Parallel (FPP)\n");
  

•                   break;
                    /* Add new interface types here */
  

•           case fast_passive_parallel_security:
  

•                   printf ("Fast Passive Parallel with Security (FPPS) \n");
  

•                   break;
  

•                   /* Add new interface types here */
            default:
                    printf ("Unsupported interface type, %d\n", desc->iface);
            }
  

@@ -166,6 +200,14 @@ __FUNCTION__); #endif break;

•                   case Altera_StratixII:
  

+#if (CONFIG_FPGA & CFG_STRATIX_II)

•                           StratixII_info (desc);
  

+#else

•                           printf ("%s: No support for Stratix II  devices.\n",
  

•                                      __FUNCTION__);
  

+#endif

Ditto: dump the else clause

ok.

...

•                           break;
                            /* Add new family types here */
                    default:
                            /* we don't need a message here - we give one up above */
  

@@ -199,6 +241,16 @@ __FUNCTION__); #endif break;

•           case Altera_StratixII:
  

+#if (CONFIG_FPGA & CFG_STRATIX_II)

•                   ret_val = StratixII_reloc (desc, reloc_offset);
  

+#else

•                   printf ("%s: No support for Stratix II devices.\n",
  

•                           __FUNCTION__);
  

+#endif

•                   break;
  

•           case Altera_CYC2:
  

#if (CONFIG_FPGA & CFG_CYCLON2) ret_val = CYC2_reloc (desc, reloc_offset); @@ -219,7 +271,7 @@

/* ------------------------------------------------------------------------- */

-static int altera_validate (Altera_desc * desc, char *fn) +static int altera_validate (Altera_desc * desc, const char *fn) { int ret_val = FALSE;

@@ -249,4 +301,4 @@

/* ------------------------------------------------------------------------- */

-#endif /* CONFIG_FPGA & CFG_FPGA_ALTERA */ +#endif /* CONFIG_FPGA & CFG_FPGA_ALTERA */

Unneeded whitespace churn

dont get this one. what do you mean?

...

Index: include/altera.h

--- include/altera.h (.../tags/trunk/20070620_2_merge_to_exsw6000) (revision 69) +++ include/altera.h (.../branches/exsw6000) (revision 69) @@ -27,22 +27,21 @@ #ifndef _ALTERA_H_ #define _ALTERA_H_

-/*

• • See include/xilinx.h for another working example.
• */

/* Altera Model definitions *********************************************************************/ #define CFG_ACEX1K CFG_FPGA_DEV( 0x1 ) #define CFG_CYCLON2 CFG_FPGA_DEV( 0x2 ) +#define CFG_STRATIX_II CFG_FPGA_DEV( 0x4 )

#define CFG_ALTERA_ACEX1K (CFG_FPGA_ALTERA | CFG_ACEX1K) #define CFG_ALTERA_CYCLON2 (CFG_FPGA_ALTERA | CFG_CYCLON2) +#define CFG_ALTERA_STRATIX_II (CFG_FPGA_ALTERA | CFG_STRATIX_II) /* Add new models here */

/* Altera Interface definitions *********************************************************************/ #define CFG_ALTERA_IF_PS CFG_FPGA_IF( 0x1 ) /* passive serial */ +#define CFG_ALTERA_IF_FPP CFG_FPGA_IF( 0x2 ) /* fast passive parallel */ /* Add new interfaces here */

typedef enum { /* typedef Altera_iface */ @@ -52,6 +51,8 @@ passive_parallel_asynchronous, /* parallel data */ passive_serial_asynchronous, /* serial data w/ internal clock (not used) */ altera_jtag_mode, /* jtag/tap serial (not used ) */

•   fast_passive_parallel,  /* fast passive parallel (FPP) */
  

•   fast_passive_parallel_security, /* fast passive parallel with security (FPPS) */
    max_altera_iface_type           /* insert all new types before this */
  

} Altera_iface; /* end, typedef Altera_iface */

@@ -59,6 +60,7 @@ min_altera_type, /* insert all new types after this */ Altera_ACEX1K, /* ACEX1K Family */ Altera_CYC2, /* CYCLONII Family */

•   Altera_StratixII,       /* StratixII Familiy */
  

/* Add new models here */ max_altera_type /* insert all new types before this */ } Altera_Family; /* end, typedef Altera_Family */ @@ -91,4 +93,15 @@ typedef int (*Altera_abort_fn)( int cookie ); typedef int (*Altera_post_fn)( int cookie );

+typedef struct {

•   Altera_pre_fn pre;
  

•   Altera_config_fn config;
  

•   Altera_status_fn status;
  

•   Altera_done_fn done;
  

•   Altera_clk_fn clk;
  

•   Altera_data_fn data;
  

•   Altera_abort_fn abort;
  

•   Altera_post_fn post;
  

+} altera_board_specific_func;

#endif /* _ALTERA_H_ */ Index: common/stratixII.c =================================================================== --- common/stratixII.c (.../tags/trunk/20070620_2_merge_to_exsw6000) (revision 0) +++ common/stratixII.c (.../branches/exsw6000) (revision 69) @@ -0,0 +1,213 @@ +/*

• • (C) Copyright 2007
• • Eran Liberty, Extricom , eran.liberty@gmail.com
• • 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 <common.h> /* core U-Boot definitions */ +#include <altera.h>

+#if (CONFIG_FPGA & (CFG_ALTERA | CFG_STRATIX_II))

+int StratixII_fpp_load (Altera_desc * desc, void *buf, size_t bsize,

•                   int secure);
  

+int StratixII_fpp_dump (Altera_desc * desc, void *buf, size_t bsize);

+/****************************************************************/ +/* Stratix II Generic Implementation */ +int StratixII_load (Altera_desc * desc, void *buf, size_t bsize) +{

•   int ret_val = FPGA_FAIL;
  

•   switch (desc->iface) {
  

•   case fast_passive_parallel:
  

•           ret_val = StratixII_fpp_load (desc, buf, bsize, 0);
  

•           break;
  

•   case fast_passive_parallel_security:
  

•           ret_val = StratixII_fpp_load (desc, buf, bsize, 1);
  

•           break;
  

•           /* Add new interface types here */
  

•   default:
  

•           printf ("%s: Unsupported interface type, %d\n", __FUNCTION__,
  

•                   desc->iface);
  

•   }
  

•   return ret_val;
  

+}

+int StratixII_dump (Altera_desc * desc, void *buf, size_t bsize) +{

•   int ret_val = FPGA_FAIL;
  

•   switch (desc->iface) {
  

•   case fast_passive_parallel:
  

•   case fast_passive_parallel_security:
  

•           ret_val = StratixII_fpp_dump (desc, buf, bsize);
  

•           break;
  

•           /* Add new interface types here */
  

•   default:
  

•           printf ("%s: Unsupported interface type, %d\n", __FUNCTION__,
  

•                   desc->iface);
  

•   }
  

•   return ret_val;
  

+}

+int StratixII_info (Altera_desc * desc) +{

•   return FPGA_SUCCESS;
  

+}

+int StratixII_reloc (Altera_desc * desc, ulong reloc_offset) +{

•   int i;
  

•   void **func_table;
  

•   desc->iface_fns = (void *)((ulong) (desc->iface_fns) + reloc_offset);
  

•   for (i = 0, func_table = (void **)desc->iface_fns;
  

•        i < sizeof (altera_board_specific_func) / sizeof (void *); i++) {
  

•           func_table[i] =
  

•               (void *)((ulong) (func_table[i]) + reloc_offset);
  

•   }
  

•   return FPGA_SUCCESS;
  

+}

+#endif

+/************************************************************/ +/* Stratix II Fast Passive Parallel Generic Implementation */ +int StratixII_fpp_dump (Altera_desc * desc, void *buf, size_t bsize) +{

•   printf ("Stratix II Fast Passive Parallel dump is not implemented\n");
  

•   return FPGA_FAIL;
  

+}

+int StratixII_fpp_load (Altera_desc * desc, void *buf, size_t bsize, int secure) +{

•   altera_board_specific_func *fns;
  

•   int cookie;
  

•   int ret_val = FPGA_FAIL;
  

•   int bytecount;
  

•   char *buff = buf;
  

•   int i;
  

•   if (!desc) {
  

•           printf ("%s(%d) Altera_desc missing\n", __FUNCTION__, __LINE__);
  

•           return FPGA_FAIL;
  

•   }
  

•   if (!buff) {
  

•           printf ("%s(%d) buffer is missing\n", __FUNCTION__, __LINE__);
  

•           return FPGA_FAIL;
  

•   }
  

•   if (!bsize) {
  

•           printf ("%s(%d) size is zero\n", __FUNCTION__, __LINE__);
  

•           return FPGA_FAIL;
  

•   }
  

•   if (!desc->iface_fns) {
  

•           printf
  

•               ("%s(%d) Altera_desc function interface table is missing\n",
  

•                __FUNCTION__, __LINE__);
  

•           return FPGA_FAIL;
  

•   }
  

•   fns = (altera_board_specific_func *) (desc->iface_fns);
  

•   cookie = desc->cookie;
  

•   if (!
  

•       (fns->config && fns->status && fns->done && fns->data
  

•        && fns->abort)) {
  

•           printf
  

•               ("%s(%d) Missing some function in the function interface table\n",
  

•                __FUNCTION__, __LINE__);
  

•           return FPGA_FAIL;
  

•   }
  

•   /* 1. give board specific a chance to do anything before we start */
  

•   if (fns->pre) {
  

•           if ((ret_val = fns->pre (cookie)) < 0) {
  

•                   return ret_val;
  

•           }
  

•   }
  

•   /* from this point on we must fail gracfully by calling lower layer abort */
  

•   /* 2. Strat burn cycle by deasserting config for t_CFG and waiting t_CF2CK after reaserted */
  

•   fns->config (0, 1, cookie);
  

•   udelay (5);             /* nCONFIG low pulse width 2usec */
  

•   fns->config (1, 1, cookie);
  

•   udelay (100);           /* nCONFIG high to first rising edge on DCLK */
  

•   /* 3. Start the Data cycle with clk deasserted */
  

•   bytecount = 0;
  

•   fns->clk (0, 1, cookie);
  

•   printf ("loading to fpga    ");
  

•   while (bytecount < bsize) {
  

•           /* 3.1 check stratix has not signaled us an error */
  

•           if (fns->status (cookie) != 1) {
  

•                   printf
  

•                       ("\n%s(%d) Stratix failed (byte transfered till failure 0x%x)\n",
  

•                        __FUNCTION__, __LINE__, bytecount);
  

•                   fns->abort (cookie);
  

•                   return FPGA_FAIL;
  

•           }
  

•           /* 3.2 put data on the bus */
  

•           fns->data (buff[bytecount++], 1, cookie);
  

•           /* 3.3 clock once */
  

•           ndelay (5);
  

•           fns->clk (1, 1, cookie);
  

•           ndelay (5);
  

•           fns->clk (0, 1, cookie);
  

•           /* 3.4 for secure cycle push 3 more  clocks */
  

•           for (i = 0; secure && i < 3; i++) {
  

•                   ndelay (5);
  

•                   fns->clk (1, 1, cookie);
  

•                   ndelay (5);
  

•                   fns->clk (0, 1, cookie);
  

•           }
  

•           /* 3.5 while clk is deasserted it is safe to print some progress indication */
  

•           if ((bytecount % (bsize / 100)) == 0) {
  

•                   printf ("\b\b\b%02d\%", bytecount * 100 / bsize);
  

•           }
  

•   }
  

•   /* 4. Set one last clock and check conf done signal */
  

•   fns->clk (1, 1, cookie);
  

•   udelay (100);
  

•   if (!fns->done (cookie)) {
  

•           printf (" error!.\n");
  

•           fns->abort (cookie);
  

•           return FPGA_FAIL;
  

•   } else {
  

•           printf ("\b\b\b done.\n");
  

•   }
  

•   /* 5. call lower layer post configuration */
  

•   if (fns->post) {
  

•           if ((ret_val = fns->pre (cookie)) < 0) {
  

•                   fns->abort (cookie);
  

•                   return ret_val;
  

•           }
  

•   }
  

•   return FPGA_SUCCESS;
  

+} Index: include/stratixII.h =================================================================== --- include/stratixII.h (.../tags/trunk/20070620_2_merge_to_exsw6000) (revision 0) +++ include/stratixII.h (.../branches/exsw6000) (revision 69) @@ -0,0 +1,32 @@ +/*

• • (C) Copyright 2007
• • Eran Liberty, Extricom, eran.liberty@gmail.com
• • 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
• */

+#ifndef _STRATIXII_H_ +#define _STRATIXII_H_

+extern int StratixII_load (Altera_desc * desc, void *image, size_t size); +extern int StratixII_dump (Altera_desc * desc, void *buf, size_t bsize); +extern int StratixII_info (Altera_desc * desc); +extern int StratixII_reloc (Altera_desc * desc, ulong reloc_off);

+#endif /* _STRATIXII_H_ */ Index: common/Makefile =================================================================== --- common/Makefile (.../tags/trunk/20070620_2_merge_to_exsw6000) (revision 69) +++ common/Makefile (.../branches/exsw6000) (revision 69) @@ -27,7 +27,8 @@

AOBJS =

-COBJS = main.o ACEX1K.o altera.o bedbug.o circbuf.o cmd_autoscript.o \ +COBJS = main.o ACEX1K.o altera.o stratixII.o \

•     bedbug.o circbuf.o cmd_autoscript.o \
      cmd_bdinfo.o cmd_bedbug.o cmd_bmp.o cmd_boot.o cmd_bootm.o \
      cmd_cache.o cmd_console.o \
      cmd_date.o cmd_dcr.o cmd_diag.o cmd_display.o cmd_doc.o cmd_dtt.o \
  
  
  

---

This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ _______________________________________________ U-Boot-Users mailing list U-Boot-Users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/u-boot-users

-- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. grant.likely@secretlab.ca (403) 399-0195

Liberty