[U-Boot] [PATCH 3/9] openrisc: Add cpu files

[Mike Frysinger <vapier@gentoo.org>]

On Saturday 19 November 2011 00:21:32 Stefan Kristiansson wrote:
> --- /dev/null
> +++ b/arch/openrisc/config.mk
>
> \ No newline at end of file

might want to fix that

> --- /dev/null
> +++ b/arch/openrisc/cpu/cache.c
>
> +int checkicache(void)
> +int checkdcache(void)

these should be static

> +void dcache_enable(void)
> +{
> +	mtspr(SPR_SR, mfspr(SPR_SR) | SPR_SR_DCE);
> +	asm("l.nop");
> +	asm("l.nop");
> +	asm("l.nop");
> +	asm("l.nop");
> +	asm("l.nop");
> +	asm("l.nop");
> +	asm("l.nop");
> +	asm("l.nop");
> +}
> +
> +void icache_enable(void)
> +{
> +	mtspr(SPR_SR, mfspr(SPR_SR) | SPR_SR_ICE);
> +	asm("l.nop");
> +	asm("l.nop");
> +	asm("l.nop");
> +	asm("l.nop");
> +	asm("l.nop");
> +	asm("l.nop");
> +	asm("l.nop");
> +	asm("l.nop");
> +}

the lack of volatile and the lack of any sort of constraints means gcc is free 
to throw that away ...

> --- /dev/null
> +++ b/arch/openrisc/cpu/cpu.c
>
> +void illegal_instruction_handler(void)
> +void checkinstructions(void)
> +int checkcpu(void)

looks like these should be static

> +extern void __reset(void);
> +
> +int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> +	disable_interrupts();
> +	__reset();
> +	return 0;
> +}

__reset() should not return

> --- /dev/null
> +++ b/arch/openrisc/cpu/exceptions.c
>
> +extern void hang(void);

common.h already has a prototype for this; delete it.

> +static void exception_hang(int vect)
> +{
> +	printf("Unhandled exception at 0x%x ", vect & 0xff00);
> +	switch (vect & 0xff00) {
> +	case 0x100:
> +		puts("(Reset)\n");
> +		break;
> +	case 0x200:
> +		puts("(Bus Error)\n");
> +		break;
> +	case 0x300:
> +		puts("(Data Page Fault)\n");
> +		break;
> +	case 0x400:
> +		puts("(Instruction Page Fault)\n");
> +		break;
> +	case 0x500:
> +		puts("(Tick Timer)\n");
> +		break;
> +	case 0x600:
> +		puts("(Alignment)\n");
> +		break;
> +	case 0x700:
> +		puts("(Illegal Instruction)\n");
> +		break;
> +	case 0x800:
> +		puts("(External Interrupt)\n");
> +		break;
> +	case 0x900:
> +		puts("(D-TLB Miss)\n");
> +		break;
> +	case 0xa00:
> +		puts("(I-TLB Miss)\n");
> +		break;
> +	case 0xb00:
> +		puts("(Range)\n");
> +		break;
> +	case 0xc00:
> +		puts("(System Call)\n");
> +		break;
> +	case 0xd00:
> +		puts("(Floating Point)\n");
> +		break;
> +	case 0xe00:
> +		puts("(Trap)\n");
> +		break;
> +	default:
> +		puts("(Unknown exception)\n");
> +		break;
> +	}

looks like this could easily be a table string lookup:
static const char * const excp_table[] = {
	"Reset",
	"Bus Error",
	...
};
printf("(%s)\n", excp_table[(vect >> 16) & 0xff]);

> --- /dev/null
> +++ b/arch/openrisc/cpu/interrupts.c
>
> +#include <asm/types.h>
> +#include <asm/ptrace.h>
> +#include <asm/system.h>
> +#include <asm/openrisc_exc.h>
> +#include <common.h>

asm/ includes should come after non-asm/ includes
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20111119/411f2c20/attachment.pgp>