[Top] [All Lists]

Re: [PATCH] IDT Interprise Processor Support for Linux 2.6.x

To: "Tiwari, Rakesh" <>
Subject: Re: [PATCH] IDT Interprise Processor Support for Linux 2.6.x
From: Chris Wedgwood <>
Date: Fri, 10 Mar 2006 09:45:35 -0800
Cc: 'Ralf Baechle' <>,
In-reply-to: <>
Original-recipient: rfc822;
References: <>
Additional comments:

  * Firstly, it's really great to see this!

  * A single 1.6MB patch is far from ideal, please try to break it
    into a series of smaller logically separate patches.  It's hard to
    comment on a giant patch.  Perhaps something like:
      - a patch for each CPU
      - a patch for each driver
      - a patch for each platform/eval-board
    and see what you have left. Each patch should have a suitable
    description.  Also put "Signed-off-by:" lines on your patches.

  * You shouldn't be removing .gitignore :-)

  * The Ethernet drivers should probably go and cc

  * The code contains unreferenced functions?  Without even looking
    hard I can see rc32434_mii_handler is declared and not used for

  * It might be that some of the CPU-level code should be platform
    level.  For example having two UARTs is a feature of the EB434 not
    the rc32434 so EB434_UART1_IRQ is misplaced I would argue.

  * Some init code should probably be declared __init and similar

  * There is quite a bit of extraneous white-space that could be
    cleaned up and some minor indentation cleanups to match what is
    elsewhere in the kernel.

Sorry this is a little vague and 'hand-wavy', if you post smaller
logically complete patches I think you'll get better feedback where
people can comment more easily.  Ideally inline to the email if you
can, m$ lookout/$exchange as that just makes a mess, if you have to
use that then attach them as you did.

<Prev in Thread] Current Thread [Next in Thread>