Re: [PATCH 05/24] C6X: early boot code
From: Sam Ravnborg <hidden>
Date: 2011-08-09 19:26:48
+; +#include <linux/linkage.h> +#include <linux/of_fdt.h> +#include <asm/asm-offsets.h> +
I would expect this code to be part of the head section: Use __HEAD
quoted hunk ↗ jump to hunk
+ENTRY(_c_int00) + ;; Save magic and pointer + MV .S1 A4,A10 + MV .S2 B4,B10 + MVKL .S2 __bss_start,B5 + MVKH .S2 __bss_start,B5 + MVKL .S2 __bss_stop,B6 + MVKH .S2 __bss_stop,B6 + SUB .L2 B6,B5,B6 ; bss size + + ;; Set the stack pointer + MVKL .S2 current_ksp,B0 + MVKH .S2 current_ksp,B0 + LDW .D2T2 *B0,B15 + + ;; clear bss + SHR .S2 B6,3,B0 ; number of dwords to clear + ZERO .L2 B13 + ZERO .L2 B12 +bss_loop: + BDEC .S2 bss_loop,B0 + NOP 3 + CMPLT .L2 B0,0,B1 + [!B1] STDW .D2T2 B13:B12,*B5++[1] + + NOP 4 + AND .D2 ~7,B15,B15 + + ;; Clear GIE and PGIE + MVC .S2 CSR,B2 + CLR .S2 B2,0,1,B2 + MVC .S2 B2,CSR + MVC .S2 TSR,B2 + CLR .S2 B2,0,1,B2 + MVC .S2 B2,TSR + MVC .S2 ITSR,B2 + CLR .S2 B2,0,1,B2 + MVC .S2 B2,ITSR + MVC .S2 NTSR,B2 + CLR .S2 B2,0,1,B2 + MVC .S2 B2,NTSR + + ;; pass DTB pointer to machine_init (or zero if none) + MVKL .S1 OF_DT_HEADER,A0 + MVKH .S1 OF_DT_HEADER,A0 + CMPEQ .L1 A10,A0,A0 + [A0] MV .S1X B10,A4 + [!A0] MVK .S1 0,A4 + +#ifdef CONFIG_BIG_KERNEL + MVKL .S1 machine_init,A0 + MVKH .S1 machine_init,A0 + B .S2X A0 + ADDKPC .S2 0f,B3,4 +0: +#else + CALLP .S2 machine_init,B3 +#endif + + ;; Jump to Linux init +#ifdef CONFIG_BIG_KERNEL + MVKL .S1 start_kernel,A0 + MVKH .S1 start_kernel,A0 + B .S2X A0 +#else + B .S2 start_kernel +#endif + NOP 5 +L1: BNOP .S2 L1,5diff --git a/arch/c6x/kernel/setup.c b/arch/c6x/kernel/setup.c +#include <linux/of.h> +#include <linux/of_fdt.h> +#include <linux/kernel.h> +#include <linux/sched.h> +#include <linux/delay.h> +#include <linux/interrupt.h> +#include <linux/fs.h> +#include <linux/genhd.h> +#include <linux/errno.h> +#include <linux/console.h> +#include <linux/string.h> +#include <linux/delay.h> +#include <linux/bootmem.h> +#include <linux/seq_file.h> +#include <linux/module.h> +#include <linux/init.h> +#include <linux/mm.h> +#include <linux/clk.h> +#include <linux/io.h> +#include <linux/cache.h> +#include <linux/dma-mapping.h> +#include <linux/memblock.h>
For C files we sometimes use "inverse christmas tree" sort for include lists. That is: longest lines first. And sorted alphabetically within blocks of equal length. This would also make it obvious had you had two identical include files.
+#include <asm/setup.h> +#include <asm/irq.h> +#include <asm/machdep.h> +#include <asm/sections.h> +#include <asm/soc.h> + +#ifdef CONFIG_BLK_DEV_INITRD +#include <linux/initrd.h> +#include <asm/pgtable.h> +#endif
Do these include really need to be guarded?
+ +#ifdef DEBUG +#define DBG(fmt...) printk(KERN_ERR fmt) +#else +#define DBG(fmt...) +#endif
Drop this. It is used in one place - which I think is not a debug info anyway.
+ /* What can we do if we didn't find ? */
+ if (machine_id >= &__machine_desc_end) {
+ DBG("No suitable machine found !\n");
+ for (;;)
+ ;We stop in a never ending loop. So debug seems not appropriate...