lkml.org 
[lkml]   [2008]   [Apr]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: Microblaze Linux release
    Date
    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 <><


    \
     
     \ /
      Last update: 2008-04-14 08:07    [W:5.067 / U:0.096 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site