
Dear Daniel Hellstrom,
In message 1274194143-8994-3-git-send-email-daniel@gaisler.com you wrote:
Signed-off-by: Daniel Hellstrom daniel@gaisler.com
...
- (C) Copyright 2007
- Daniel Hellstrom, Gaisler Research, daniel@gaisler.com
- (C) Copyright 2010
- Daniel Hellstrom, Aeroflex Gaisler, daniel@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