[coreboot] [RFC] Proposal for policy for changing the development guidelines

Sun Jan 31 00:05:58 CET 2016

On 01/30/2016 11:30 AM, ron minnich wrote:
> The change to the guidelines is hence a codification of a practice that
> goes back to the project's beginnings.

I find that statement inaccurate. This practice had not been an issue in
the past, on patches that you yourself approved. Mixed syntaxes have
been present in the codebase for a while. See EXHIBIT A.

EXHIBIT B shows a patch, mid last year, when a new intel_syntax code was
introduced. There was only one comment (following) about the syntax. The
patch was approved by you, Ron, without mentioning what you claim has
been a practice.

> Patrick Georgi, Jul 13, 2015
> follow up patch: we don't really want two _syntaxes in one file_, right?
(emphasis added)

The comment clearly addresses the issue of mixing two syntaxes within a
file. It does not mention "a practice that goes back to the project's
beginnings", nor does it mention that either "AT&T syntax is to be used
through-out the project", or "No new Intel syntax code is allowed into
the project."

EXHIBIT C, shows another such patch that, you, have also approved, Ron.
The comment got no replies.

> Ronald G. Minnich, Oct 30 10:43 AM
> why intel syntax _in a file that is att syntax_? it's a real
oportunity for a buggy time.+2'ing because you promised me you're
rewrite it.
(emphasis added)

Notice how the issue here is strictly the mixing of syntaxes within the
same file.

EXHIBIT D shows another such patch, this time approved by me. I have
made a comment about the issue of mixed syntax.

> Alexandru Gagniuc, Jul 17, 2015
> Really? Mixed syntax in the same file?

This also addresses strictly the issue of mixing of syntaxes within the
same file.

EXHIBIT E shows another patch, that you yourself approved. This time,
there was not even a mention of mixed syntaxes.

I conclude from these, that your assertion that this has been an
unspoken rule, is not true. Furthermore, I believe that this arbitrary
change was done as an act of spite towards a set of engineers. Also,
since you, and the rest of the coreboot leadership have shown a pattern
of first discussing changes to the project publicly, on the mailing
list, I conclude that a discussion about this issue was deliberately
avoided in order to prevent those engineers, as well as other coreboot
community members, from presenting their arguments.

Alex

### EXHIBIT A: Presence of .intel_syntax ###

* 4a30ab9 cpu/amd: remove .intel_syntax
* 0302b06 arch/x86: remove .intel_syntax
* c7b2b7c cbfstool: Fix potential error when using hash attribute

$ git reset --hard c7b2b7c67dc8afb52c3dc8e9297e5ed81fa22674

$  git grep intel_syntax
src/arch/x86/boot.c:            ".intel_syntax noprefix\n\t"
src/arch/x86/c_start.S:.intel_syntax noprefix
src/arch/x86/wakeup.S:  .intel_syntax noprefix
src/cpu/amd/agesa/cache_as_ram.inc:  .intel_syntax noprefix
src/cpu/amd/pi/cache_as_ram.inc:  .intel_syntax noprefix

### EXHIBIT B: src/arch/x86/[boot.c, c_start.S] ###

$ git blame src/arch/x86/boot.c |grep intel_syntax
9693885a src/arch/x86/boot/boot.c  (Stefan Reinauer 2015-06-18 01:23:48
-0700  63)              ".intel_syntax noprefix\n\t"

$ git blame src/arch/x86/c_start.S |grep intel_syntax
9693885a src/arch/x86/lib/c_start.S  (Stefan Reinauer
2015-06-18 01:23:48 -0700 403) .intel_syntax noprefix

$ git show 9693885a
commit 9693885ad88d21ead7bd9ebc32f3e4901841b18b
Author: Stefan Reinauer <stefan.reinauer at coreboot.org>
Date:   Thu Jun 18 01:23:48 2015 -0700

    x86: Port x86 over to compile cleanly with x86-64

    Change-Id: I26f1bbf027435be593f11bce4780111dcaf7cb86
    Signed-off-by: Stefan Reinauer <stefan.reinauer at coreboot.org>
    Signed-off-by: Scott Duplichan <scott at notabs.org>
    Reviewed-on: http://review.coreboot.org/10586
    Tested-by: build bot (Jenkins)
    Tested-by: Raptor Engineering Automated Test Stand
<noreply at raptorengineeringinc.com>
    Reviewed-by: Ronald G. Minnich <rminnich at gmail.com>

### EXHIBIT C: src/arch/x86/wakeup.S ###

$ git blame src/arch/x86/wakeup.S |grep intel_syntax
ac901e6b src/arch/x86/wakeup.S       (Stefan Reinauer 2015-07-31
16:46:28 -0700  32)    .intel_syntax noprefix

$ git show ac901e6b
commit ac901e6bedc98d115ebb8089afd8f67aef39c000
Author: Stefan Reinauer <reinauer at chromium.org>
Date:   Fri Jul 31 16:46:28 2015 -0700

    wakeup: Switch back to 32bit mode first

    On x86_64 we need to leave long mode before we can switch to 16bit
    mode. Oh joy! When's my 64bit resume pointer coming?

    Why didn't this get caught earlier? Seems the Asrock E350M2 didn't
    do Suspend/Resume?

    Yes, I know it's Intel syntax. Will be converted to AT&T syntax
    as soon as the whole thing actually works.. 8)

    Change-Id: Ic51869cf67d842041f8842cd9964d72a024c335f
    Signed-off-by: Stefan Reinauer <stefan.reinauer at coreboot.org>
    Reviewed-on: http://review.coreboot.org/11106
    Tested-by: build bot (Jenkins)
    Reviewed-by: Ronald G. Minnich <rminnich at gmail.com>

### EXHIBIT D: src/cpu/amd/agesa/cache_as_ram.inc ###

$ git blame src/cpu/amd/agesa/cache_as_ram.inc |grep intel_syntax
67b9430b src/cpu/amd/agesa/cache_as_ram.inc          (Stefan Reinauer
2015-06-18 01:14:01 -0700  66)   .intel_syntax noprefix

$ git show 67b9430b
commit 67b9430b367a9f9a884043f14365a55b7ef3c45c
Author: Stefan Reinauer <stefan.reinauer at coreboot.org>
Date:   Thu Jun 18 01:14:01 2015 -0700

    cpu: port amd/agesa to 64bit

    Change-Id: I8644b04f4b57db5fc95ec155d3f78d53c63c9831
    Signed-off-by: Stefan Reinauer <stefan.reinauer at coreboot.org>
    Signed-off-by: Scott Duplichan <scott at notabs.org>
    Reviewed-on: http://review.coreboot.org/10579
    Tested-by: build bot (Jenkins)
    Reviewed-by: Alexandru Gagniuc <mr.nuke.me at gmail.com>
    Tested-by: Raptor Engineering Automated Test Stand
<noreply at raptorengineeringinc.com>

### EXHIBIT E: src/cpu/amd/pi/cache_as_ram.inc ###

$ git blame src/cpu/amd/pi/cache_as_ram.inc |grep intel_syntax
f7613eca (Stefan Reinauer 2015-07-21 13:34:01 -0700  67)   .intel_syntax
noprefix

$ git show f7613eca
commit f7613ecacc02d486caa868a1f494dc46983aeb3d
Author: Stefan Reinauer <reinauer at chromium.org>
Date:   Tue Jul 21 13:34:01 2015 -0700

    cpu: port amd/pi to 64bit

    Change-Id: I66ef081fa1a520f0199366587800783ea1ef8719
    Signed-off-by: Stefan Reinauer <stefan.reinauer at coreboot.org>
    Reviewed-on: http://review.coreboot.org/11023
    Reviewed-by: Ronald G. Minnich <rminnich at gmail.com>
    Tested-by: build bot (Jenkins)