[U-Boot,v3,1/8] x86: Move table csum into separate header

We need the checksum function without all the other table functionality
soon, so let's split it out into its own header file.

Signed-off-by: Alexander Graf <agraf@suse.de>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
---
 arch/x86/include/asm/tables.h |  2 ++
 arch/x86/lib/tables.c         | 12 ------------
 include/tables_csum.h         | 22 ++++++++++++++++++++++
 3 files changed, 24 insertions(+), 12 deletions(-)
 create mode 100644 include/tables_csum.h

Hi Alex,

On 16 August 2016 at 02:55, Alexander Graf <agraf@suse.de> wrote:
> We need the checksum function without all the other table functionality
> soon, so let's split it out into its own header file.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>  arch/x86/include/asm/tables.h |  2 ++
>  arch/x86/lib/tables.c         | 12 ------------
>  include/tables_csum.h         | 22 ++++++++++++++++++++++
>  3 files changed, 24 insertions(+), 12 deletions(-)
>  create mode 100644 include/tables_csum.h

Reviewed-by: Simon Glass <sjg@chromium.org>

Question below

>
> diff --git a/arch/x86/include/asm/tables.h b/arch/x86/include/asm/tables.h
> index ae9f0d0..81f98f2 100644
> --- a/arch/x86/include/asm/tables.h
> +++ b/arch/x86/include/asm/tables.h
> @@ -7,6 +7,8 @@
>  #ifndef _X86_TABLES_H_
>  #define _X86_TABLES_H_
>
> +#include <tables_csum.h>
> +
>  /*
>   * All x86 tables happen to like the address range from 0xf0000 to 0x100000.
>   * We use 0xf0000 as the starting address to store those tables, including
> diff --git a/arch/x86/lib/tables.c b/arch/x86/lib/tables.c
> index f92111e..9ee6b5e 100644
> --- a/arch/x86/lib/tables.c
> +++ b/arch/x86/lib/tables.c
> @@ -38,18 +38,6 @@ static table_write table_write_funcs[] = {
>  #endif
>  };
>
> -u8 table_compute_checksum(void *v, int len)
> -{
> -       u8 *bytes = v;
> -       u8 checksum = 0;
> -       int i;
> -
> -       for (i = 0; i < len; i++)
> -               checksum -= bytes[i];
> -
> -       return checksum;
> -}
> -
>  void table_fill_string(char *dest, const char *src, size_t n, char pad)
>  {
>         int start, len;
> diff --git a/include/tables_csum.h b/include/tables_csum.h
> new file mode 100644
> index 0000000..27d147b
> --- /dev/null
> +++ b/include/tables_csum.h
> @@ -0,0 +1,22 @@
> +/*
> + * Copyright (C) 2015, Bin Meng <bmeng.cn@gmail.com>
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#ifndef _TABLES_CSUM_H_
> +#define _TABLES_CSUM_H_
> +
> +static inline u8 table_compute_checksum(void *v, int len)

Why is this static inline? Shouldn't it go in a C file?

> +{
> +       u8 *bytes = v;
> +       u8 checksum = 0;
> +       int i;
> +
> +       for (i = 0; i < len; i++)
> +               checksum -= bytes[i];
> +
> +       return checksum;
> +}
> +
> +#endif
> --
> 1.8.5.6
>

Regards,
Simon

> On 17 Aug 2016, at 06:15, Simon Glass <sjg@chromium.org> wrote:
> 
> Hi Alex,
> 
> On 16 August 2016 at 02:55, Alexander Graf <agraf@suse.de> wrote:
>> We need the checksum function without all the other table functionality
>> soon, so let's split it out into its own header file.
>> 
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
>> ---
>> arch/x86/include/asm/tables.h |  2 ++
>> arch/x86/lib/tables.c         | 12 ------------
>> include/tables_csum.h         | 22 ++++++++++++++++++++++
>> 3 files changed, 24 insertions(+), 12 deletions(-)
>> create mode 100644 include/tables_csum.h
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> Question below
> 
>> 
>> diff --git a/arch/x86/include/asm/tables.h b/arch/x86/include/asm/tables.h
>> index ae9f0d0..81f98f2 100644
>> --- a/arch/x86/include/asm/tables.h
>> +++ b/arch/x86/include/asm/tables.h
>> @@ -7,6 +7,8 @@
>> #ifndef _X86_TABLES_H_
>> #define _X86_TABLES_H_
>> 
>> +#include <tables_csum.h>
>> +
>> /*
>>  * All x86 tables happen to like the address range from 0xf0000 to 0x100000.
>>  * We use 0xf0000 as the starting address to store those tables, including
>> diff --git a/arch/x86/lib/tables.c b/arch/x86/lib/tables.c
>> index f92111e..9ee6b5e 100644
>> --- a/arch/x86/lib/tables.c
>> +++ b/arch/x86/lib/tables.c
>> @@ -38,18 +38,6 @@ static table_write table_write_funcs[] = {
>> #endif
>> };
>> 
>> -u8 table_compute_checksum(void *v, int len)
>> -{
>> -       u8 *bytes = v;
>> -       u8 checksum = 0;
>> -       int i;
>> -
>> -       for (i = 0; i < len; i++)
>> -               checksum -= bytes[i];
>> -
>> -       return checksum;
>> -}
>> -
>> void table_fill_string(char *dest, const char *src, size_t n, char pad)
>> {
>>        int start, len;
>> diff --git a/include/tables_csum.h b/include/tables_csum.h
>> new file mode 100644
>> index 0000000..27d147b
>> --- /dev/null
>> +++ b/include/tables_csum.h
>> @@ -0,0 +1,22 @@
>> +/*
>> + * Copyright (C) 2015, Bin Meng <bmeng.cn@gmail.com>
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#ifndef _TABLES_CSUM_H_
>> +#define _TABLES_CSUM_H_
>> +
>> +static inline u8 table_compute_checksum(void *v, int len)
> 
> Why is this static inline? Shouldn't it go in a C file?

It could go into a C file as well, but I didn’t see many downsides of having it be a static inline. Would you prefer it in a C file?


Alex

Hi Alex,

On 17 August 2016 at 00:27, Alexander Graf <agraf@suse.de> wrote:
>
>> On 17 Aug 2016, at 06:15, Simon Glass <sjg@chromium.org> wrote:
>>
>> Hi Alex,
>>
>> On 16 August 2016 at 02:55, Alexander Graf <agraf@suse.de> wrote:
>>> We need the checksum function without all the other table functionality
>>> soon, so let's split it out into its own header file.
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
>>> ---
>>> arch/x86/include/asm/tables.h |  2 ++
>>> arch/x86/lib/tables.c         | 12 ------------
>>> include/tables_csum.h         | 22 ++++++++++++++++++++++
>>> 3 files changed, 24 insertions(+), 12 deletions(-)
>>> create mode 100644 include/tables_csum.h
>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>
>> Question below
>>
>>>
>>> diff --git a/arch/x86/include/asm/tables.h b/arch/x86/include/asm/tables.h
>>> index ae9f0d0..81f98f2 100644
>>> --- a/arch/x86/include/asm/tables.h
>>> +++ b/arch/x86/include/asm/tables.h
>>> @@ -7,6 +7,8 @@
>>> #ifndef _X86_TABLES_H_
>>> #define _X86_TABLES_H_
>>>
>>> +#include <tables_csum.h>
>>> +
>>> /*
>>>  * All x86 tables happen to like the address range from 0xf0000 to 0x100000.
>>>  * We use 0xf0000 as the starting address to store those tables, including
>>> diff --git a/arch/x86/lib/tables.c b/arch/x86/lib/tables.c
>>> index f92111e..9ee6b5e 100644
>>> --- a/arch/x86/lib/tables.c
>>> +++ b/arch/x86/lib/tables.c
>>> @@ -38,18 +38,6 @@ static table_write table_write_funcs[] = {
>>> #endif
>>> };
>>>
>>> -u8 table_compute_checksum(void *v, int len)
>>> -{
>>> -       u8 *bytes = v;
>>> -       u8 checksum = 0;
>>> -       int i;
>>> -
>>> -       for (i = 0; i < len; i++)
>>> -               checksum -= bytes[i];
>>> -
>>> -       return checksum;
>>> -}
>>> -
>>> void table_fill_string(char *dest, const char *src, size_t n, char pad)
>>> {
>>>        int start, len;
>>> diff --git a/include/tables_csum.h b/include/tables_csum.h
>>> new file mode 100644
>>> index 0000000..27d147b
>>> --- /dev/null
>>> +++ b/include/tables_csum.h
>>> @@ -0,0 +1,22 @@
>>> +/*
>>> + * Copyright (C) 2015, Bin Meng <bmeng.cn@gmail.com>
>>> + *
>>> + * SPDX-License-Identifier:    GPL-2.0+
>>> + */
>>> +
>>> +#ifndef _TABLES_CSUM_H_
>>> +#define _TABLES_CSUM_H_
>>> +
>>> +static inline u8 table_compute_checksum(void *v, int len)
>>
>> Why is this static inline? Shouldn't it go in a C file?
>
> It could go into a C file as well, but I didn’t see many downsides of having it be a static inline. Would you prefer it in a C file?

Yes I think so. We normally put C code in C files. But Bin may have comments.

Regards,
Simon