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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 jeff@xxxxxxxxxx and cc
    netdev@xxxxxxxxxxxxxxx

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

  * 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.


[Index of Archives]     [Linux MIPS Home]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Linux]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux