[Skiboot] Barreleye platform code for skiboot
jk at ozlabs.org
Mon Feb 29 12:36:32 AEDT 2016
Thanks for your contribution to the skiboot project!
Overall, the platform definition for barreleye looks good. I've put a
few comments against sections of the patch below.
However, it's not usual to send contributions as a password-protected
zip file to an open source project. Instead, just a plain-text patch
included inline (ie, instead of an attachment) makes things a lot easier
for the reviewers and maintainers.
Since you're probably using a git tree for skiboot, there are some git
commands you can use to automatically format your change into an
- `git format-patch <commit-range>`
this turns all of the commits in <commit-range> into an email,
using the proper Subject and changelog entries from the
<commit-range> can be something like "origin/master..master"
(which specifies all commits between the upstream master branch
and your local master branch), or just "-1" (which specifies
just the last one commit).
- `git send-email --to <recipient> <files>`
given a set of files produced by format-patch, send them using a
preconfigured mail server (see `git send-email --help` for details
on specifying a SMTP server).
I generally use something like:
git format-patch origin/master..HEAD
git send-email --to skiboot at lists.ozlabs.org *.patch
I understand that your company may not be able to send emails directly;
but at least using an inline-patch format, rather than attachments,
might help here. We can probably help you out to find a method of
making this easy for you.
You'll also need a signed-off-by line, to certify that your contribution
adheres to the developers certificate of origin (ie, that you have the
authorisation to submit the code to this project). See
http://developercertificate.org/ for details. That will look
Signed-off-by: Jeremy Kerr <jk at ozlabs.org>
(but with the actual author's name, of course :) )
Here's an example of a correctly-formatted contribution:
Note how the patch is included inline, and the commit log is included as
part of the submission.
From your patch:
> +++ b/platforms/astbmc/barreleye.c
> @@ -0,0 +1,174 @@
> +/* Copyright 2013-2014 IBM Corp.
> + *
You probably want to at least add Foxconn as a copyright holder here.
> +static bool barreleye_probe(void)
> + if (!dt_node_is_compatible(dt_root, "foxconn,barreleye"))
> + return false;
This looks like the first usage of the "foxconn," vendor prefix in a
device tree. This will become the standard vendor prefix for future
device-tree binding definitions from your company. Some companies use
their US stock-ticker name, and some use their full name, as you have
The prefix "foxconn" looks fine to me, but you should ensure that it's
future-proof for more contributions.
I'm assuming that the slot names are all correct. If so, the slot map
also looks good to me.
More information about the Skiboot