Hi,
Looking much better now, thanks.
Few suggestion, not usually well documented, but a convention nevertheless:
$subject: might be a good idea to do the following for revision of patch
original patch [PATCH] is considered v1
next revision such as this is considered v2 [PATCH v2] is useful for
people to keep track of the revision of patch under discussion.
when you post with git send-email especially in a follow on patch, add
--in-reply-to "Original patch subject" - this helps mail readers like
gmail/thunderbird etc to properly group them up for the receiver.
Honestly, I forget to do this most of the time, but is a good practice
to have nonetheless.
srinath@xxxxxxxxxxxxxxxxxxxx had written, on 10/25/2010 09:45 AM, the
following:
From: Srinath <srinath@xxxxxxxxxxxxxxxxxxxx>
This patch adds basic board file. Detailed support will follow in
subsequent patches.
[1] http://www.ti.com/sitara
[2] http://www.mistralsolutions.com/products/craneboard.php
Look at other board file addition examples - for example:
IGEP - commit ID: 58e111621d402d41cb0cabae7c532d6194b7d943
it gives a brief description of
a) what the board is
b) overall resemblance to other boards
c) idea about the family of silicon involved
when you give a link - we in the linux-omap probably have heard about
sitara family of processors, but the patch needs to go down to linux-arm
and lkml - where progressively, the processor is not probably not that
well heard of :). it might be nice to state OMAP3 family of processors
to give the reviewer an idea about what is being talked about.
http://omapedia.org/wiki/Releasing_to_Linux_kernel_using_patches_and_emails#Note_on_git_commit_comments
See the point on "Suggestions for Commit messages:"
This patch has been created against omap-next branch.
This is not something you'd like to see in kernel.org commit message
right? this is the type of info which could easily fit into diffstat
section.
Signed-off-by: Srinath <srinath@xxxxxxxxxxxxxxxxxxxx>
---
this side of the patch is called the diffstat - >from here to the first
diff does not get into the git history when committed with git am - so
we can add quiet a few things here
Since this is v2 of the patch series, few suggestions
a) "This patch has been created against omap-next branch" is nice thing
to do -> keep in mind, with omap-for-linus being merged to kernel.org,
you may want to base further changes on kernel.org - but if you state
something to the maintainer, this is the place to do it
b) when you post v2 of a patch, keep in mind you'd like to help a reader
who might have missed reviewing your previous patch to review this one
in context to the previous patches as well.. at least the rule I try to
follow is to have the diffstat of a later series contain details as follows:
v2: change 1
change 2
did not do change 3 - reason why..
v1: link of the discussion, I prefer using marc.info as it can give me a
complete thread easily, for example, I'd post a link like this:
http://marc.info/?t=128801458500004&r=1&w=2
Reason for doing this:
a) having additional reviewers improves your code
b) for an old reviewer, v2 sections tell him/her what has changed and he
can focus to those changes alone, rest he has already seen
b) for a new reviewer, v1 gives a context of previous discussion, v2
tells him/her what actions where taken by the author as a result and
provide comments in reference to that.
Note: this is a bit of a pain as the patch goes through multiple
iterations, but as far as I saw, it has at least helped my patches at
times :).
arch/arm/configs/omap2plus_defconfig | 1 +
arch/arm/mach-omap2/Kconfig | 5 ++
arch/arm/mach-omap2/Makefile | 2 +
arch/arm/mach-omap2/board-am3517crane.c | 70 ++++++++++++++++++++++++++
arch/arm/plat-omap/include/plat/uncompress.h | 1 +
5 files changed, 79 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-omap2/board-am3517crane.c
diff --git a/arch/arm/configs/omap2plus_defconfig b/arch/arm/configs/omap2plus_defconfig
index ccedde1..8c93f86 100644
--- a/arch/arm/configs/omap2plus_defconfig
+++ b/arch/arm/configs/omap2plus_defconfig
@@ -40,6 +40,7 @@ CONFIG_MACH_OMAP_LDP=y
CONFIG_MACH_OVERO=y
CONFIG_MACH_OMAP3EVM=y
CONFIG_MACH_OMAP3517EVM=y
+CONFIG_MACH_CRANEBOARD=y
CONFIG_MACH_OMAP3_PANDORA=y
CONFIG_MACH_OMAP3_TOUCHBOOK=y
CONFIG_MACH_OMAP_3430SDP=y
I suggest you drop this change -> leave it to Tony when he creates his
usual defconfig cleanups :)
look at commit ids:
58e111621d402d41cb0cabae7c532d6194b7d943 - IGEP board
b075f58b2c0f377b4bfbe11b817e003393bcb489 - PandaBoard
for examples how the board introductions have been done recently.
[...]
rest looks ok to me.
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
