Messages in this thread | | | From | Arnd Bergmann <> | Subject | Re: Microblaze Linux release | Date | Mon, 14 Apr 2008 08:01:55 +0200 |
| |
On Monday 14 April 2008, Arnd Bergmann wrote:
> I'm certainly willing to help review it, but I think the timing is too short > for asking for a 2.6.26 merge. Judging from experience, a new architecture > review takes a few weeks to sort out all the issues you want resolved before > merging, so posting them now would put you in a much more comfortable position > when targeting a 2.6.27 merge.
I've just looked at your git tree, and it confirmed my point that we need a little more time. Please don't hesitate to submit patches to the mailing list though. You may have misunderstood the idea of the merge window: The patches can go in during that time if they have been reviewed before, so it is not the time to post patches for review.
More to the point, here are a few things that I noticed on a brief review:
* Your arch code in general looks nice and clean, don't worry too much about the coding style right now.
* The code level is 2.6.24-rc8, while the current head is at 2.6.25-rc9. You should really upgrade the kernel level, because a number of changes have gone in that directly affect your code. Your best bet is probably now to have a patch against the linux-next tree, which also has the changes that will go into 2.6.26.
* Your syscall ABI is largely obsolete. A third of the syscalls you define should not even be there in the first place as they have been replaced by newer versions. E.g. you have select, _newselect and pselect6, where just pselect6 would be sufficient -- you don't need to worry about backwards compatibility if you don't have existing code. A good start would be to take the arch/blackfin syscall list and further reduce it from there. Further examples are: - replace socketcall with separate syscall entry points - replace ipc with a separate entry points - remove all the legacy signal handling from arch/microblaze/kernel/signal.c - remove sys_mmap, sys_olduname and sys_vfork - finally define a generic sys_mmap2 and sys_uname in kernel/ so you don't need another copy in arch/microblaze/kernel - Use 64 bit off_t, and 32 bit uid_t, gid_t etc.
* Your device tree functions in of_device.c, of_platform.c, prom.c and prom_parse.c are basically copies of the powerpc versions. This duplication is not helpful, better merge that code into new files in drivers/of/ so that it can be shared.
* The semaphore implementation will be obsolete in 2.6.26
* The EXPORT_SYMBOL definitions for csum_partial etc should be in arch/microblaze/lib/checksum.c, not in microblaze_ksyms.c, same for memcpy/memmove/memset.
* The platform directory is noticably empty. You can probably judge better how much platform specific code there will be in the future, but I would suspect that you're better off with a flat platform directory instead of subdirectories below that.
* the consistent.c implementation looks strange. Is any of that code even used anywhere? What is it good for, considering that you don't have an mmu?
* Your dma_mapping.h does BUG() for almost everything. I suspect you could easily replace it with a trivial implementation, like the x86_32 one.
* I'm not sure if you really need something as sophisticated as the lmb code, if you don't have a real firmware, or even an MMU. setup_memory can probably work with something much simpler, and for the prom.c stuff, you may get away with #defining them to bootmem_alloc etc.
* You don't seem to have any code for PCI, so the pci.h and pci-bridge.h files don't make much sense.
* The following files: atomic.h checksum.h io.h ioctl.h ioctls.h ipcbuf.h msgbuf.h param.h poll.h posix_types.h sembuf.h shmbuf.h shmparam.h sigcontext.h signal.h socket.h sockios.h stat.h termbits.h termios.h types.h ucontext.h all look like verbatim copies of some other architecture. Please do the next person to come up with a new architecture a favor and stick them in include/asm-generic instead, including them from your own headers.
* You unistd.h contains _syscall* macros. These were removed some time ago from all other architectures.
* You define a lot of __ARCH_WANT_SYS_ macros. Generally, these are for the syscalls you don't want.
* You are missing a include/asm-microblaze/Kbuild file that lists all the header files you want to export to user space.
Arnd <><
| |