Dear Daniel Hellstrom, In message <1274194143-8994-3-git-send-email-dan...@gaisler.com> you wrote: > Signed-off-by: Daniel Hellstrom <dan...@gaisler.com> ... > - * (C) Copyright 2007 > - * Daniel Hellstrom, Gaisler Research, dan...@gaisler.com > + * (C) Copyright 2010 > + * Daniel Hellstrom, Aeroflex Gaisler, dan...@gaisler.com.
Thsi cannot be. Make this "Copyright 2007, 2010" or "Copyright 2007-2010". > * Foundation, Inc., 59 Temple Place, Suite 330, Boston, > * MA 02111-1307 USA > + * Please drop this empty line. > +/* #define DEBUG */ > + Please do not add dead code. ... > + int i; > + > + ambapp_find_buses(ioarea, abus); > + for(i=0; i<6; i++) > + if ( abus->ioareas[i] == 0 ) > + break; Multiline if's require braces. Please also fix white space: for (i=0; i<6; i++) { if (abus->ioareas[i] == 0) break; } > + if ( type == AMBA_TYPE_AHBIO ) { Please fix white spaces as above. Please fix this globally. > + if ( found == 1 ) { > + ambapp_apb_parse(&apbdev, dev); > + } No braces for one liners. And fix the white space. ... > + found = ambapp_find_ahb(abus, devid, 63, type, &ahbdev); > + if ( found == 1 ) { > + return 64; > + } else { > + return 63 - ahbdev.dec_index; > } Seems I have seen semi-identical code above. Factor out as function? > +/* The define CONFIG_SYS_GRLIB_SINGLE_BUS may be defined on GRLIB systems > + * where only one AHB Bus is available - no bridges are present. This option > + * is available only to reduce the footprint. > + * > + * Defining this on a multi-bus GRLIB system may also work depending on the > + * design. > + */ Incorrect multiline comment style. > +/* Get Parent bus frequency. Note that since we go from a "child" bus > + * to a parent bus, the frequency factor direction is inverted. > + */ Incorrect multiline comment style. Please fix globally. > + if ( dir ) { > + freq = freq * ffact; > + } else { > + freq = freq / ffact; > + } No braces for oneliners. Please fix globally. > +unsigned int gaisler_ahb2ahb_v2_freq(ambapp_ahbdev *ahb, unsigned int freq) > { > - /* find first device of this */ > - return ambapp_ahb_scan(vendor, driver, dev, index, 1, AHB_SCAN_SLAVE); > + printf("gaisler_ahb2ahb_v2_freq: AHB2AHB ver 2 not supported\n"); > + while(1) ; Consider calling hang() instead ? [globally] ... > + /* Get parent bus */ > + parent = (ioarea & 0x7); > + if ( parent == 0 ) { > + printf("ambapp_bus_freq: parent=0 indicates no parent! > Stopping.\n"); Line too long. Please fix globally. ... > +init_ahb_bridges: > + mov %g0, %i0 > + mov %g0, %i1 > + mov %g0, %i2 > + mov %g0, %i3 > + mov %g0, %i4 > + retl > + mov %g0, %i5 Incorrect indentation. Please fix globally. ... > void cpu_init_f(void) > { > - /* these varaiable must not be initialized */ > - ambapp_dev_irqmp *irqmp; > - ambapp_apbdev apbdev; > - register unsigned int apbmst; > - > - /* find AMBA APB Master */ > - apbmst = (unsigned int) > - ambapp_ahb_next_nomem(VENDOR_GAISLER, GAISLER_APBMST, 1, 0); > - if (!apbmst) { > - /* > - * no AHB/APB bridge, something is wrong > - * ==> jump to start (or hang) > - */ > - while (1) ; > - } > - /* Init memory controller */ > - if (init_memory_ctrl()) { > - while (1) ; > - } > - > - /**************************************************** > - * From here we can use the main memory and the stack. > - */ > > - /* Find AMBA APB IRQMP Controller */ > - if (ambapp_apb_first(VENDOR_GAISLER, GAISLER_IRQMP, &apbdev) != 1) { > - /* no IRQ controller, something is wrong > - * ==> jump to start (or hang) > - */ > - while (1) ; > - } > - irqmp = (ambapp_dev_irqmp *) apbdev.address; > - > - /* initialize the IRQMP */ > - irqmp->ilevel = 0xf; /* all IRQ off */ > - irqmp->iforce = 0; > - irqmp->ipend = 0; > - irqmp->iclear = 0xfffe; /* clear all old pending interrupts */ > - irqmp->cpu_mask[0] = 0; /* mask all IRQs on CPU 0 */ > - irqmp->cpu_force[0] = 0; /* no force IRQ on CPU 0 */ > - > - /* cache */ > } Seems this becomes an empty function? Drop it, then! > +/* Routine called from start.S, > + * > + * Run from FLASH/PROM: > + * - memory controller has already been setup up, stack can be used > + * - global variables available for read/writing > + * - constants avaiable > + */ > void cpu_init_f2(void) > { > - > + /* Initialize the AMBA Plug & Play bus structure, the bus > + * structure represents the AMBA bus that the CPU is located at. > + */ > + ambapp_bus_init(CONFIG_AMBAPP_IOAREA, CONFIG_SYS_CLK_FREQ, &ambapp_plb); > } Or rename this into cpu_init_f() ? > +/** Vendor GAISLER devices */ > +static ambapp_device_name GAISLER_devices[] = > +{ > + {GAISLER_LEON2DSU, "LEON2DSU", "Leon2 Debug Support Unit"}, > + {GAISLER_LEON3, "LEON3", "Leon3 SPARC V8 Processor"}, > + {GAISLER_LEON3DSU, "LEON3DSU", "Leon3 Debug Support Unit"}, > + {GAISLER_ETHAHB, "ETHAHB", "OC ethernet AHB interface"}, ... Please use TABs for indentation - fix globally, please. 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 I haven't lost my mind -- it's backed up on tape somewhere. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot