[U-Boot] [PATCH v2 02/11] x86: Move ipchecksum into common area and rename it

[Simon Glass]

Hi Bin,

On 10 January 2015 at 20:04, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Simon,
>
> On Sun, Jan 11, 2015 at 11:33 AM, Simon Glass <sjg at chromium.org> wrote:
>> Hi Bin,
>>
>> On 10 January 2015 at 19:44, Bin Meng <bmeng.cn at gmail.com> wrote:
>>>
>>> Hi Simon,
>>>
>>> On Sun, Jan 11, 2015 at 1:20 AM, Simon Glass <sjg at chromium.org> wrote:
>>> > Hi Bin,
>>> >
>>> > On 10 January 2015 at 08:47, Bin Meng <bmeng.cn at gmail.com> wrote:
>>> >> Hi Simon,
>>> >>
>>> >> On Fri, Jan 9, 2015 at 11:30 AM, Simon Glass <sjg at chromium.org> wrote:
>>> >>> The existing IP checksum function is only accessible to the 'coreboot' cpu.
>>> >>> Move it into the common area and rename it slightly to remove the
>>> >>> abbreviations.
>>> >>>
>>> >>> Signed-off-by: Simon Glass <sjg at chromium.org>
>>> >>> ---
>>> >>>
>>> >>> Changes in v2:
>>> >>> - Refactor IP checksum patches
>>> >>>
>>> >>>  arch/x86/cpu/Makefile                           |  1 +
>>> >>>  arch/x86/cpu/coreboot/Makefile                  |  1 -
>>> >>>  arch/x86/cpu/coreboot/ipchecksum.c              | 55 -------------------------
>>> >>>  arch/x86/cpu/coreboot/tables.c                  |  8 ++--
>>> >>>  arch/x86/cpu/ip_checksum.c                      | 34 +++++++++++++++
>>> >>
>>> >> What about the namings we discussed in the v1 patch thread? ip_xxx
>>> >> indicates ip protocol, but acutally they are not.
>>> >
>>> > Is it not? This is not actually CMOS-specific - e.g. it is used by
>>> > Coreboot to send data through to U-Boot in the tables it provides.
>>> >
>>> > I thought it was an IP checksum...
>>> >
>>>
>>> OK, so you mean coreboot is using the same IP checksum algorithm to
>>> generate checksums for coreboot tables to be passed to U-Boot. If that
>>> is the case, I think we should put the these files to u-boot/net or
>>> u-boot/lib. Are there existing codes that we can reuse?
>>
>> NetCkSum() is similar but does not need to worry about an odd length.
>> So I doubt I can use that.
>>
>
> But would you consider moving it to some another place more suitable
> than arch/x86/cpu/? I feel that it is IP related stuff and it may make
> people confused when seeing it in arch/x86 directory. If we move it to
> u-boot/net, maybe you can replace NetCkSum() with the one we have here
> since it can deal with odd length.

I suspect it might be confusing in any case. But I think we can move
this file to net/ - at least I don't see any big problems. I wonder
whether people might complain about changing the existing NetChkSum()
function, but we will see.

Regards,
Simon