From: Olof Johansson <olof@lixom.net>
To: Nicolas DET <nd@bplan-gmbh.de>
Cc: linuxppc-dev@ozlabs.org, tnt@246tNt.com, sl@bplan-gmbh.de
Subject: Re: [PATCH] General CHRP/MPC5K2 Platform and drivers support - to comment
Date: Thu, 19 Oct 2006 12:43:29 -0500 [thread overview]
Message-ID: <20061019124329.1f32257a@pb15> (raw)
In-Reply-To: <453771E5.4090808@bplan-gmbh.de>
On Thu, 19 Oct 2006 14:39:01 +0200 Nicolas DET <nd@bplan-gmbh.de> 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
next prev parent reply other threads:[~2006-10-19 17:47 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-10-19 12:39 [PATCH] General CHRP/MPC5K2 Platform and drivers support - to comment Nicolas DET
2006-10-19 14:12 ` Jon Loeliger
2006-10-19 16:18 ` Linas Vepstas
2006-10-19 16:26 ` Grant Likely
2006-10-19 23:41 ` Benjamin Herrenschmidt
2006-10-20 5:55 ` Kumar Gala
2006-10-19 17:43 ` Olof Johansson [this message]
2006-10-20 6:06 ` Kumar Gala
2006-10-20 6:29 ` Sven Luther
2006-10-20 6:29 ` Benjamin Herrenschmidt
2006-10-20 8:12 ` Nicolas DET
2006-10-20 14:18 ` Kumar Gala
2006-10-20 14:24 ` Nicolas DET
2006-10-20 14:34 ` Kumar Gala
2006-10-21 23:25 ` Benjamin Herrenschmidt
2006-10-22 12:56 ` Nicolas DET
2006-10-23 6:36 ` Benjamin Herrenschmidt
2006-10-23 14:47 ` Matt Sealey
2006-10-23 15:52 ` Christoph Hellwig
2006-10-23 16:25 ` Matt Sealey
2006-10-23 16:40 ` Segher Boessenkool
2006-10-23 21:38 ` Benjamin Herrenschmidt
2006-10-23 22:57 ` Paul Mackerras
2006-10-24 15:44 ` Matt Sealey
2006-10-24 16:01 ` Becky Bruce
2006-10-24 23:01 ` Paul Mackerras
2006-10-25 14:44 ` Matt Sealey
2006-10-25 14:50 ` Kumar Gala
2006-10-25 14:56 ` Matt Sealey
2006-10-25 14:57 ` Grant Likely
2006-10-25 16:18 ` Matt Sealey
2006-10-25 17:08 ` Olof Johansson
2006-10-25 21:29 ` Paul Mackerras
2006-10-25 21:22 ` Paul Mackerras
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20061019124329.1f32257a@pb15 \
--to=olof@lixom.net \
--cc=linuxppc-dev@ozlabs.org \
--cc=nd@bplan-gmbh.de \
--cc=sl@bplan-gmbh.de \
--cc=tnt@246tNt.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).