[PATCH v2] arm64: enable EDAC on arm64

Rob Herring robherring2 at gmail.com
Thu Dec 5 08:52:23 EST 2013


On Thu, Dec 5, 2013 at 5:51 AM, Catalin Marinas <catalin.marinas at arm.com> wrote:
> On Thu, Dec 05, 2013 at 12:53:33AM +0000, Rob Herring wrote:
>> On Tue, Nov 26, 2013 at 9:37 AM, Catalin Marinas
>> <catalin.marinas at arm.com> wrote:
>> > On Fri, Nov 22, 2013 at 09:05:08PM +0000, Rob Herring wrote:
>> >> +static inline void atomic_scrub(void *va, u32 size)
>> >> +{
>> >> +     unsigned int *virt_addr = va;
>> >> +     unsigned int i;
>> >> +
>> >> +     for (i = 0; i < size / sizeof(*virt_addr); i++, virt_addr++) {
>> >
>> > BTW, maybe the compiler is smart enough to drop the i but why not just
>> > use a int *last_addr = va + size - 3; and just compare against this?
>>
>> Evidently, the compiler is not that smart. Here's what I ended up
>> with. There's no need to subtract 3 that I can see:
>>
>> unsigned int *last_addr = va + size;
>> for (; virt_addr < last_addr; virt_addr++) {
>
> The 3 thing was for size not multiple of 4. Do we have any guarantees
> here?

Only that no one does ECC on less that 64-bit wide DDR. I'm only
assuming 32-bit alignment and that is what all the other
implementations assume.

If we have a non-multiple of 4 size, then we want to expand the size
to include the partial bytes. This function is a nop effectively, so
the fact that we may touch 1-3 extra bytes at the end does not matter.
The only thing subtracting 3 does is mean the comparison needs to be
'<=' instead of '<'. It would also break if you had a size of 1 or 2.
I think what you intended was to align size, but you really need to
add 3 and mask it to do that. But that is not necessary in my version.

Rob



More information about the linux-arm-kernel mailing list