Re: [PATCH 2/7] clean up apus support
From: Paul Mackerras <hidden>
Date: 2004-05-27 11:04:51
Roman Zippel writes:
This brings the APUS into 2.6 and cleans up a lot of indirect calls via the mach_* functions and removes amiga/ints.c, amiga/time.c and the the reference to <asm-m68k/machdep.h>.
Some comments:
quoted hunk ↗ jump to hunk
--- 2.6/arch/ppc/amiga/Makefile 31 Jan 2004 23:23:58 -0000 1.1.1.1 +++ 2.6/arch/ppc/amiga/Makefile 24 May 2004 23:12:48 -0000@@ -2,7 +2,7 @@ # Makefile for Linux arch/m68k/amiga source directory
It would be nice to fix that comment.
quoted hunk ↗ jump to hunk
+++ 2.6/arch/ppc/platforms/apus_setup.c Wed Feb 4 13:21:35 2004
...
+int amiga_hwclk(int, struct rtc_time *); +int amiga_set_clock_mmss (unsigned long nowtime);
I really don't like seeing function prototypes in a .c file for functions that are actually in another .c file. I understand the temptation but it is a source of bugs. Please move these declarations to a header which gets included in both the file where they are defined and the file where they are used.
+ ciab.cra = (ciab.cra & 0xc0) | 0x08; + ciab.icr; + wmb(); + ciab.talo = 0; + wmb(); + ciab.tahi = 0x80; + wmb();
What the heck is this? Is this some sort of access to an I/O device? Are you assuming a 1-1 virtual<->physical mapping to make this work? This looks to me like it should be using some of the in/out macros instead of explicitly putting in wmb. The other patches all look fine to me. Paul. ** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/