linux-mips
[Top] [All Lists]

[PATCH 2.6.16-rc5 1/5] IDT Interprise Processor Support for Linu x 2.6.

To: "'Chris Wedgwood'" <cw@f00f.org>
Subject: [PATCH 2.6.16-rc5 1/5] IDT Interprise Processor Support for Linu x 2.6.x
From: "Tiwari, Rakesh" <Rakesh.Tiwari@idt.com>
Date: Tue, 14 Mar 2006 16:23:50 -0800
Cc: "'Ralf Baechle'" <ralf@linux-mips.org>, "'linux-mips@linux-mips.org'" <linux-mips@linux-mips.org>
Original-recipient: rfc822;linux-mips@linux-mips.org
Sender: linux-mips-bounce@linux-mips.org

Hi Chris/All,

The attached patch adds support for the IDT Interprise processor RC32438
based on the MIPS 4KC core.

Additional information regarding IDT processor's can be found at
http://www.idt.com/?catID=58532

Signed-off-by: Rakesh Tiwari <rischelp@idt.com>

Thx
Rakesh

<<idt_rc32438.patch>>

     -----Original Message-----
    From:   Tiwari, Rakesh 
    Sent:   Monday, March 13, 2006 1:07 PM
    To:     'Chris Wedgwood'; Tiwari, Rakesh
    Cc:     'Ralf Baechle'; linux-mips@linux-mips.org
    Subject:        RE: [PATCH] IDT Interprise Processor Support for Linux  2.6.x

    Hi Chris,

    Appreciate your feedback.
    Please see my comments below, inline prefixed by [rkt]

    Look forward for additional comments/suggestions, if any.

    Thanks
    Rakesh


    -----Original Message-----
    From: Chris Wedgwood [mailto:cw@f00f.org]
    Sent: Friday, March 10, 2006 9:46 AM
    To: Tiwari, Rakesh
    Cc: 'Ralf Baechle'; linux-mips@linux-mips.org
    Subject: Re: [PATCH] IDT Interprise Processor Support for Linux 2.6.x


    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.

        [rkt] Agreed, 1.6MB is a huge patch. I will try to break it down into
              multiple patches (ard 5) based on platform/eval board and will
              send it out soon.

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

        [rkt] I think these are still there.
           
      * The Ethernet drivers should probably go jeff@garzik.org and cc
        netdev@vger.kernel.org

        [rkt] The Ethernet interface/driver is integral to each processor
        and dependent upon other system header files, unlike a regular NIC.
        I can try posting the patches (probably sub-patch) to Jeff and netdev,
        in order to get feedback on the driver.

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

        [rkt] Chris you hit the bulls eye. This is the only function which
        I missed out... Will clean it up.
           
      * 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.

        [rkt] Since all the IDT's processors are primarily SoC's, the UARTS are
         part of the processor. In case on rc32434 there is only 1 UART. However
         rc32438 has 2 UARTS

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

       [rkt] Will try to clean up as much as possible...

    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.

Attachment: idt_rc32438.patch
Description: Binary data

<Prev in Thread] Current Thread [Next in Thread>
  • [PATCH 2.6.16-rc5 1/5] IDT Interprise Processor Support for Linu x 2.6.x, Tiwari, Rakesh <=