Hello Paul,

On 16/03/2015 17:54, Paul Walmsley wrote:

On Mon, 16 Mar 2015, Mason wrote:

quoted

On 15/03/2015 18:40, Mason wrote:

quoted

On 13/03/2015 17:45, Russell King - ARM Linux wrote:

quoted

Yes, this one I like - and it probably fixes a potential latent bug
where the compiler was free to re-order that mrc outside of the if()
statement.

Please wrap it up as a normal submission, thanks.

Proposed patch at the end of this message.

I'm now puzzling over why it's required to have "memory"
in read_cpuid_ext's clobber list, and not in read_cpuid's?

Reviewed-by: Paul Walmsley <paul@pwsan.com>

Looks reasonable to me.  I'd suggest updating the patch message to 
describe your change, and why it's needed.  Consider something like:

---

Convert the open-coded MMFR0 register read in __get_cpu_architecture() to 
use the read_cpuid_ext() macro.  This shortens the function and ensures 
that a memory clobber is used on the coprocessor read instruction.  The 
memory clobber works around a bug in gcc 4.5.  gcc 4.5 can reorder 
coprocessor read instructions with respect to other code, disregarding 
potential side-effects of the coprocessor read.

To be honest, the reason I wrote the patch in the first place
was merely to fix the code duplication! ;-)

I wasn't aware of the latent-bug issue until Russel mentioned
it. So I didn't want to put too much emphasis on that part,
since it didn't come from me, and it is well-documented in
your own commit, which I referenced.

Do you know why it was necessary to fix read_cpuid_ext and
not read_cpuid? I would think that the same problem affects
both macros.

Once you've got something that you're happy with, and have reposted it to 
the public lists, I believe the next step will be for you to post it to 
rmk's patch tracker at:

http://www.arm.linux.org.uk/developer/patches/

Oh, I didn't know about that part. It's not mentioned in
https://www.kernel.org/doc/Documentation/SubmittingPatches

Thanks for the review, and for mentioning the tracker.

Ah yes, now I see this:
http://www.arm.linux.org.uk/mailinglists/faq.php#p1

Will post an (hopefully) improved commit message ASAP.

Regards.