[Skiboot] ***UNCHECKED*** Barreleye platform code for skiboot

Stewart Smith stewart at linux.vnet.ibm.com
Mon Feb 29 13:20:53 AEDT 2016


johnny.cl.chang at foxconn.com writes:
> <DIV style="FONT-SIZE: 10pt; FONT-FAMILY: Microsoft Sans Serif">Hello OZlabs,</DIV>
> <DIV style="FONT-SIZE: 10pt; FONT-FAMILY: Microsoft Sans Serif"> </DIV>
> <DIV style="FONT-SIZE: 10pt; FONT-FAMILY: Microsoft Sans Serif"> </DIV>
> <DIV style="FONT-SIZE: 10pt; FONT-FAMILY: Microsoft Sans Serif"> </DIV>
> <DIV style="FONT-SIZE: 10pt; FONT-FAMILY: Microsoft Sans Serif">This is Foxconn BIOS team.</DIV>
> <DIV style="FONT-SIZE: 10pt; FONT-FAMILY: Microsoft Sans Serif"> </DIV>
> <DIV style="FONT-SIZE: 10pt; FONT-FAMILY: Microsoft Sans Serif">We are trying to merge code to upstream for Barreleye, and we need your help.</DIV>
> <DIV style="FONT-SIZE: 10pt; FONT-FAMILY: Microsoft Sans Serif"> </DIV>
> <DIV style="FONT-SIZE: 10pt; FONT-FAMILY: Microsoft Sans Serif">Could you help us to review this code patch and commit to upstream.</DIV>
> <DIV style="FONT-SIZE: 10pt; FONT-FAMILY: Microsoft Sans Serif"> </DIV>
> <DIV style="FONT-SIZE: 10pt; FONT-FAMILY: Microsoft Sans Serif">The patch is attached and unzip password is 123456</DIV>
> <DIV style="FONT-SIZE: 10pt; FONT-FAMILY: Microsoft Sans Serif"> </DIV>
> <DIV style="FONT-SIZE: 10pt; FONT-FAMILY: Microsoft Sans Serif">Thanks.<BR><BR></DIV>
> <DIV style="FONT-SIZE: 10pt; FONT-FAMILY: Microsoft Sans Serif"><BR>Johnny</DIV>
> <DIV style="FONT-SIZE: 10pt; FONT-FAMILY: Microsoft Sans Serif"><BR>mail from ip-->192.168.0.100<BR>mail from pc-->DESKTOP-6QBJCE7<BR>Version: Super Notes 1.6.5.9B</DIV>
> <DIV style="FONT-SIZE: 10pt; FONT-FAMILY: Microsoft Sans
> Serif"><BR></DIV>

Hi!

Firstly, thanks for your patch. I have a couple of points of feedback
that need to be addressed before we can merge it, some generic and some
specific to the patch:

- your email client seems to have messed up the text/plain and text/html
  parts and swapped them, creating the mess you see above. Generally,
  when posting to free software mailing lists, text/plain only is
  preferred, we don't need text/html parts at all.
- Please post patches inline rather than as attachments, especially
  zipped ones with a password. This way they get picked up automatically
  by various systems (such as patchwork) and enable easier review and
  status monitoring for both submitter and maintainers.
- Your patch is missing a Signed-Off-By line.
  See https://www.kernel.org/doc/Documentation/SubmittingPatches as we
  use the same process as the kernel for skiboot (especially see section 11)
  The signed-off-by line indicates that you have permission to post the
  patch and acts as an audit trail of where the patch has come from,
  who's reviewed it etc.

Specific points of feedback:
- The (C) header on platforms/astbmc/barreleye.c - I doubt that it
  really was 2013-2014 (C) IBM Corp. Likely most of this code was
  developed by non-IBM and thus should carry appropriate (C) header
  (incl correct year)

Apart from those points, it looks pretty good (I'm assuming the slot
tables are accurate, I haven't checked any barreleye documentation to
verify their accuracy).

If you could please send a V2 of the patch addressing the above issues,
I think we could incorporate it very soon.

thanks,
-- 
Stewart Smith
OPAL Architect, IBM.



More information about the Skiboot mailing list