[U-Boot] [PATCH 02/10] x86: Add checksum implementation

[Simon Glass]

Hi Bin,

On 3 January 2015 at 23:16, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Simon,
>
> On Tue, Dec 30, 2014 at 9:12 AM, Simon Glass <sjg at chromium.org> wrote:
>> Add a checksum implementation that can be used with CMOS RAM.
>
> What about the existing coreboot version of checksum implementation
> (arch/x86/cpu/coreboot/ipchecksum.c)? We need consolidate them.
>
>> Signed-off-by: Simon Glass <sjg at chromium.org>
>> ---
>>
>>  arch/x86/cpu/Makefile              |  1 +
>>  arch/x86/cpu/ip_checksum.c         | 61 ++++++++++++++++++++++++++++++++++++++
>>  arch/x86/include/asm/ip_checksum.h | 16 ++++++++++
>>  3 files changed, 78 insertions(+)
>>  create mode 100644 arch/x86/cpu/ip_checksum.c
>>  create mode 100644 arch/x86/include/asm/ip_checksum.h
>>
>> diff --git a/arch/x86/cpu/Makefile b/arch/x86/cpu/Makefile
>> index 62e43c0..eee2289 100644
>> --- a/arch/x86/cpu/Makefile
>> +++ b/arch/x86/cpu/Makefile
>> @@ -16,6 +16,7 @@ obj-$(CONFIG_SYS_COREBOOT) += coreboot/
>>  obj-$(CONFIG_NORTHBRIDGE_INTEL_SANDYBRIDGE) += ivybridge/
>>  obj-$(CONFIG_NORTHBRIDGE_INTEL_IVYBRIDGE) += ivybridge/
>>  obj-$(CONFIG_INTEL_QUEENSBAY) += queensbay/
>> +obj-y += ip_checksum.o
>>  obj-y += lapic.o
>>  obj-y += mtrr.o
>>  obj-$(CONFIG_PCI) += pci.o
>> diff --git a/arch/x86/cpu/ip_checksum.c b/arch/x86/cpu/ip_checksum.c
>> new file mode 100644
>> index 0000000..17f9417
>> --- /dev/null
>> +++ b/arch/x86/cpu/ip_checksum.c
>
> The name of ip_checksum.c is misleading. Maybe cmos_checksum.c?
>
>> @@ -0,0 +1,61 @@
>> +/*
>> + * From Coreboot compute_ip_checksum.c
>> + *
>> + * Copyright (c) 2014 Google, Inc
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0
>> + */
>> +
>> +#include <common.h>
>> +#include <asm/ip_checksum.h>
>> +
>> +/* This function is functionally the same as NetCksum() - should we unify? */
>> +unsigned long compute_ip_checksum(void *addr, unsigned long length)
>
> compute_cmos_checksum?
>
>> +{
>> +       uint8_t *ptr;
>> +       union {
>> +               uint8_t  byte[2];
>> +               uint16_t word;
>> +       } value;
>> +       unsigned long sum;
>> +       unsigned long i;
>> +
>> +       sum = 0;
>> +       ptr = addr;
>> +       for (i = 0; i < length; i++) {
>> +               unsigned long v;
>> +
>> +               v = ptr[i];
>> +               if (i & 1)
>> +                       v <<= 8;
>> +               /* Add the new value */
>> +               sum += v;
>> +               /* Wrap around the carry */
>> +               if (sum > 0xffff)
>> +                       sum = (sum + (sum >> 16)) & 0xffff;
>> +       }
>> +       value.byte[0] = sum & 0xff;
>> +       value.byte[1] = (sum >> 8) & 0xff;
>> +       return (~value.word) & 0xffff;
>> +}
>> +
>> +unsigned long add_ip_checksums(unsigned long offset, unsigned long sum,
>> +                              unsigned long new)
>
> add_cmos_checksums?
>
>> +{
>> +       unsigned long checksum;
>> +
>> +       sum = ~sum & 0xffff;
>> +       new = ~new & 0xffff;
>> +       if (offset & 1) {
>> +               /*
>> +                * byte-swap the sum if it came from an odd offset; since the
>> +                * computation is endian independant this works.
>> +                */
>> +               new = ((new >> 8) & 0xff) | ((new << 8) & 0xff00);
>> +       }
>> +       checksum = sum + new;
>> +       if (checksum > 0xffff)
>> +               checksum -= 0xffff;
>> +
>> +       return (~checksum) & 0xffff;
>> +}
>> diff --git a/arch/x86/include/asm/ip_checksum.h b/arch/x86/include/asm/ip_checksum.h
>> new file mode 100644
>> index 0000000..6553d7b
>> --- /dev/null
>> +++ b/arch/x86/include/asm/ip_checksum.h
>> @@ -0,0 +1,16 @@
>> +/*
>> + * From Coreboot ip_checksum.h
>> + *
>> + * Copyright (c) 2014 Google, Inc
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0
>> + */
>> +
>> +#ifndef _IP_CHECKSUM_H
>> +#define _IP_CHECKSUM_H
>> +
>> +unsigned long compute_ip_checksum(void *addr, unsigned long length);
>> +unsigned long add_ip_checksums(unsigned long offset, unsigned long sum,
>> +                              unsigned long new);
>> +
>> +#endif /* IP_CHECKSUM_H */

Yes, I'll fix it. I was actually wondering about using crc16() instead.

Regards,
Simon