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@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@coreboot.org Signed-off-by: Scott Duplichan scott@notabs.org Reviewed-on: http://review.coreboot.org/10586 Tested-by: build bot (Jenkins) Tested-by: Raptor Engineering Automated Test Stand noreply@raptorengineeringinc.com Reviewed-by: Ronald G. Minnich rminnich@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@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@coreboot.org Reviewed-on: http://review.coreboot.org/11106 Tested-by: build bot (Jenkins) Reviewed-by: Ronald G. Minnich rminnich@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@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@coreboot.org Signed-off-by: Scott Duplichan scott@notabs.org Reviewed-on: http://review.coreboot.org/10579 Tested-by: build bot (Jenkins) Reviewed-by: Alexandru Gagniuc mr.nuke.me@gmail.com Tested-by: Raptor Engineering Automated Test Stand noreply@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@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@coreboot.org Reviewed-on: http://review.coreboot.org/11023 Reviewed-by: Ronald G. Minnich rminnich@gmail.com Tested-by: build bot (Jenkins)
On Sat, Jan 30, 2016 at 3:06 PM Alex G. mr.nuke.me@gmail.com wrote:
Furthermore, I believe that this arbitrary change was done as an act of spite towards a set of engineers.
Well, wow. This just got weird. I think I'm done with this discussion.
ron
* Alex G. mr.nuke.me@gmail.com [160131 00:05]:
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.
That is a really surprising statement coming from you, Alex, as you and I have discussed this very topic in person several times, and I have explained to you why I initially checked in Scott's patches with mixed syntax and we both had agreed that in the long run the code has to be rewritted in AT&T syntax. So now you are using the fact that there is no log on the mailing list about this discussion to hurt the project and create some sort of artificial divide in the community once again.
I am really disappointed in you.
Stefan
On Sun, Jan 31, 2016 at 2:24 PM, Stefan Reinauer stefan.reinauer@coreboot.org wrote:
That is a really surprising statement coming from you, Alex, as you and I have discussed this very topic in person several times
And as I have said in those very same discussions, decisions about coreboot shold be done publicly. You're also portraying a distorted picture of what was actually discussed, but it was still a private conversation and has no bearing towards what Paul pointed out.
Going back to Paul's proposal, he noted that an official project guideline had been modified with neither public mail discussion, nor community oversight. This was not a "minor typo fix" or "clarification". I count three different changes which were made this way. And I think Paul nailed it with his proposal.
I fully support Paul's proposal.
On Sat, Jan 30, 2016 at 6:54 PM, Martin Roth gaumless@gmail.com wrote:
There was significant discussion in several meetings about the reasons for and against standardizing on AT&T syntax.
As I've explained above, private conversations are not the proper forum to make decisions related to coreboot. I realize you and others involved are wearing two hats, and sometimes it's hard to tell which hat you're wearing, either for you, or observers. Please consider the image portrayed on your employer, when a group of its employees unilaterally discusses, changes, then enforces rules in a public project. I think Paul's proposal fixes this issue.
If you have a reason for using Intel syntax that is really more persuasive than keeping the asm code in the project consistent, feel free to state it.
While I have a lot to say of the matter, this is not the appropriate place. This discussion is about a change to a policy, not a development guideline. Let's focus on what Paul is saying here.
Alex
-
Alex G. -
Alex Gagniuc
-
ron minnich -
Stefan Reinauer