[U-Boot] [PATCH V3 1/3] lib_generic memcpy: copy one word at a time if possible

[Chris Moore <moore@free.fr>]

Alessandro Rubini a ?crit :
> From: Alessandro Rubini <rubini@unipv.it>
>
> If source and destination are aligned, this copies ulong values
> until possible, trailing part is copied by byte. Thanks for the details
> to Wolfgang Denk, Mike Frysinger, Peter Tyser, Chris Moore.
>
> Signed-off-by: Alessandro Rubini <rubini@unipv.it>
> Acked-by: Andrea Gallo <andrea.gallo@stericsson.com>
> ---
>  lib_generic/string.c |   19 +++++++++++++++----
>  1 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/lib_generic/string.c b/lib_generic/string.c
> index 181eda6..9bee79b 100644
> --- a/lib_generic/string.c
> +++ b/lib_generic/string.c
> @@ -446,12 +446,23 @@ char * bcopy(const char * src, char * dest, int count)
>   * You should not use this function to access IO space, use memcpy_toio()
>   * or memcpy_fromio() instead.
>   */
> -void * memcpy(void * dest,const void *src,size_t count)
> +void * memcpy(void *dest, const void *src, size_t count)
>  {
> -	char *tmp = (char *) dest, *s = (char *) src;
> -
> +	unsigned long *dl = (unsigned long *)dest, *sl = (unsigned long *)src;
>   

Nitpick: Are you sure the casts are necessary here ?
I am not sure about U-Boot but Linux kernel maintainers frown on 
unnecessary casts.

> +	char *d8, *s8;
> +
> +	/* while all data is aligned (common case), copy a word at a time */
> +	if ( (((ulong)dest | (ulong)src | count) & (sizeof(*dl) - 1)) == 0) {
>   

+	if ( (((ulong)dest | (ulong)src) & (sizeof(*dl) - 1)) == 0) {

The "or" should not include count: the remaining count % sizeof(unsigned 
long) bytes are copied below.

> +		while (count >= sizeof(*dl)) {
> +			*dl++ = *sl++;
> +			count -= sizeof(*dl);
> +		}
> +	}
> +	/* copy the reset one byte at a time */
>   

Nitpick: s/reset/rest/

> +	d8 = (char *)dl;
> +	s8 = (char *)sl;
>  	while (count--)
> -		*tmp++ = *s++;
> +		*d8++ = *s8++;
>  
>  	return dest;
>  }
>   

Cheers,
Chris