Subject: Re: [PATCH] General CHRP/MPC5K2 Platform and drivers support - to comment

From: Olof Johansson <[email-masked]>
Date: Thu, 19 Oct 2006 12:43:29 -0500

On Thu, 19 Oct 2006 14:39:01 +0200 Nicolas DET <[email-masked]> wrote:

> We know this patch is not totaly compliant with Documentation/CodingStyle/.
> 
> We would like people to comment and review it. This way we would provide 
> a new patch with the changes required (if any) for an upcomming merge in 
> the kernel.

Hi,

Like others have said, a monolithic patch like this is really hard to
review. Please split it up, there are several logical ways it could be
cut, drivers vs base platform support, etc.

First reaction from just browsing through it:

* Lots of SillyCaps, i.e. codingstyle issues, especially in the
  bestcomm code
* No need to have filenames at top of files (they're even wrong in some
  cases)
* You reinvented pr_debug() (everyone loves doing this, myself included)
* Some hardcoded baudrate stuff in the serial driver. Should probably
  be passed in from somewhere -- device tree?
* Network drivers should be posted to netdev, not the arch list

More thorough review would be easier to do with split-up patches.

-Olof