[PATCH] aarch64: add HXT Phecda core memory operation ifuncs

[PATCH] aarch64: add HXT Phecda core memory operation ifuncs

[Hongbo Zhang]

Phecda is HXT semiconductor's CPU core, this patch adds memory operation
ifuncs for it: sharing the same optimized implementation with Qualcomm's
Falkor core.

2018-06-07  Minfeng Kang <minfeng.kang@hxt-semitech.com>
	    Hongbo Zhang <hongbo.zhang@linaro.org>

	* sysdeps/aarch64/multiarch/memcpy.c (libc_ifunc): reuse
	__memcpy_falkor for phecda core.
	* sysdeps/aarch64/multiarch/memmove.c (libc_ifunc): reuse
	__memmove_falkor for phecda core.
	* sysdeps/aarch64/multiarch/memset.c (libc_ifunc): reuse
	__memset_falkor for phecda core.
	* sysdeps/unix/sysv/linux/aarch64/cpu-features.c: add MIDR entry
	for phecda core.
	* sysdeps/unix/sysv/linux/aarch64/cpu-features.h (IS_PHECDA): add
	macro to identify phecda core.
---
 sysdeps/aarch64/multiarch/memcpy.c             | 2 +-
 sysdeps/aarch64/multiarch/memmove.c            | 2 +-
 sysdeps/aarch64/multiarch/memset.c             | 7 ++++---
 sysdeps/unix/sysv/linux/aarch64/cpu-features.c | 1 +
 sysdeps/unix/sysv/linux/aarch64/cpu-features.h | 3 +++
 5 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/sysdeps/aarch64/multiarch/memcpy.c b/sysdeps/aarch64/multiarch/memcpy.c
index b94c655..4a04a63 100644
--- a/sysdeps/aarch64/multiarch/memcpy.c
+++ b/sysdeps/aarch64/multiarch/memcpy.c
@@ -36,7 +36,7 @@ extern __typeof (__redirect_memcpy) __memcpy_falkor attribute_hidden;
 libc_ifunc (__libc_memcpy,
             (IS_THUNDERX (midr)
 	     ? __memcpy_thunderx
-	     : (IS_FALKOR (midr)
+	     : (IS_FALKOR (midr) || IS_PHECDA (midr)
 		? __memcpy_falkor
 		: (IS_THUNDERX2 (midr) || IS_THUNDERX2PA (midr)
 		  ? __memcpy_thunderx2
diff --git a/sysdeps/aarch64/multiarch/memmove.c b/sysdeps/aarch64/multiarch/memmove.c
index afd8dd2..e69d816 100644
--- a/sysdeps/aarch64/multiarch/memmove.c
+++ b/sysdeps/aarch64/multiarch/memmove.c
@@ -35,7 +35,7 @@ extern __typeof (__redirect_memmove) __memmove_falkor attribute_hidden;
 libc_ifunc (__libc_memmove,
             (IS_THUNDERX (midr)
 	     ? __memmove_thunderx
-	     : (IS_FALKOR (midr)
+	     : (IS_FALKOR (midr) || IS_PHECDA (midr)
 		? __memmove_falkor
 		: __memmove_generic)));
 
diff --git a/sysdeps/aarch64/multiarch/memset.c b/sysdeps/aarch64/multiarch/memset.c
index 6df93d5..d74ed3a 100644
--- a/sysdeps/aarch64/multiarch/memset.c
+++ b/sysdeps/aarch64/multiarch/memset.c
@@ -31,9 +31,10 @@ extern __typeof (__redirect_memset) __libc_memset;
 extern __typeof (__redirect_memset) __memset_falkor attribute_hidden;
 extern __typeof (__redirect_memset) __memset_generic attribute_hidden;
 
-libc_ifunc (__libc_memset, (IS_FALKOR (midr) && zva_size == 64
-			    ? __memset_falkor
-			    : __memset_generic));
+libc_ifunc (__libc_memset,
+	    ((IS_FALKOR (midr) || IS_PHECDA (midr)) && zva_size == 64
+	     ? __memset_falkor
+	     : __memset_generic));
 
 # undef memset
 strong_alias (__libc_memset, memset);
diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
index 33b87a8..203f839 100644
--- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
+++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
@@ -35,6 +35,7 @@ static struct cpu_list cpu_list[] = {
       {"thunderxt88",	 0x430F0A10},
       {"thunderx2t99",   0x431F0AF0},
       {"thunderx2t99p1", 0x420F5160},
+      {"phecda",	 0x680F0000},
       {"generic", 	 0x0}
 };
 
diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
index cde655b..eb35adf 100644
--- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
+++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
@@ -49,6 +49,9 @@
 #define IS_FALKOR(midr) (MIDR_IMPLEMENTOR(midr) == 'Q'			      \
                         && MIDR_PARTNUM(midr) == 0xc00)
 
+#define IS_PHECDA(midr) (MIDR_IMPLEMENTOR(midr) == 'h'			      \
+                        && MIDR_PARTNUM(midr) == 0x000)
+
 struct cpu_features
 {
   uint64_t midr_el1;
-- 
2.7.4

Re: [PATCH] aarch64: add HXT Phecda core memory operation ifuncs

[Siddhesh Poyarekar]

On 06/11/2018 03:16 PM, Hongbo Zhang wrote:
> Phecda is HXT semiconductor's CPU core, this patch adds memory operation
> ifuncs for it: sharing the same optimized implementation with Qualcomm's
> Falkor core.
> 
> 2018-06-07  Minfeng Kang <minfeng.kang@hxt-semitech.com>
> 	    Hongbo Zhang <hongbo.zhang@linaro.org>
> 
> 	* sysdeps/aarch64/multiarch/memcpy.c (libc_ifunc): reuse
> 	__memcpy_falkor for phecda core.
> 	* sysdeps/aarch64/multiarch/memmove.c (libc_ifunc): reuse
> 	__memmove_falkor for phecda core.
> 	* sysdeps/aarch64/multiarch/memset.c (libc_ifunc): reuse
> 	__memset_falkor for phecda core.
> 	* sysdeps/unix/sysv/linux/aarch64/cpu-features.c: add MIDR entry
> 	for phecda core.
> 	* sysdeps/unix/sysv/linux/aarch64/cpu-features.h (IS_PHECDA): add
> 	macro to identify phecda core.

Thank you for the patch.  Your contributions are covered by the Linaro 
copyright assignment, but you have also attributed Minfeng Kang with 
part of this patch.  Can you (or Carlos/Joseph) please clarify the 
copyright assignment status for Minfeng?  I don't have access to the 
assignment file to confirm myself.

Thanks,
Siddhesh

> ---
>   sysdeps/aarch64/multiarch/memcpy.c             | 2 +-
>   sysdeps/aarch64/multiarch/memmove.c            | 2 +-
>   sysdeps/aarch64/multiarch/memset.c             | 7 ++++---
>   sysdeps/unix/sysv/linux/aarch64/cpu-features.c | 1 +
>   sysdeps/unix/sysv/linux/aarch64/cpu-features.h | 3 +++
>   5 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/sysdeps/aarch64/multiarch/memcpy.c b/sysdeps/aarch64/multiarch/memcpy.c
> index b94c655..4a04a63 100644
> --- a/sysdeps/aarch64/multiarch/memcpy.c
> +++ b/sysdeps/aarch64/multiarch/memcpy.c
> @@ -36,7 +36,7 @@ extern __typeof (__redirect_memcpy) __memcpy_falkor attribute_hidden;
>   libc_ifunc (__libc_memcpy,
>               (IS_THUNDERX (midr)
>   	     ? __memcpy_thunderx
> -	     : (IS_FALKOR (midr)
> +	     : (IS_FALKOR (midr) || IS_PHECDA (midr)
>   		? __memcpy_falkor
>   		: (IS_THUNDERX2 (midr) || IS_THUNDERX2PA (midr)
>   		  ? __memcpy_thunderx2
> diff --git a/sysdeps/aarch64/multiarch/memmove.c b/sysdeps/aarch64/multiarch/memmove.c
> index afd8dd2..e69d816 100644
> --- a/sysdeps/aarch64/multiarch/memmove.c
> +++ b/sysdeps/aarch64/multiarch/memmove.c
> @@ -35,7 +35,7 @@ extern __typeof (__redirect_memmove) __memmove_falkor attribute_hidden;
>   libc_ifunc (__libc_memmove,
>               (IS_THUNDERX (midr)
>   	     ? __memmove_thunderx
> -	     : (IS_FALKOR (midr)
> +	     : (IS_FALKOR (midr) || IS_PHECDA (midr)
>   		? __memmove_falkor
>   		: __memmove_generic)));
>   
> diff --git a/sysdeps/aarch64/multiarch/memset.c b/sysdeps/aarch64/multiarch/memset.c
> index 6df93d5..d74ed3a 100644
> --- a/sysdeps/aarch64/multiarch/memset.c
> +++ b/sysdeps/aarch64/multiarch/memset.c
> @@ -31,9 +31,10 @@ extern __typeof (__redirect_memset) __libc_memset;
>   extern __typeof (__redirect_memset) __memset_falkor attribute_hidden;
>   extern __typeof (__redirect_memset) __memset_generic attribute_hidden;
>   
> -libc_ifunc (__libc_memset, (IS_FALKOR (midr) && zva_size == 64
> -			    ? __memset_falkor
> -			    : __memset_generic));
> +libc_ifunc (__libc_memset,
> +	    ((IS_FALKOR (midr) || IS_PHECDA (midr)) && zva_size == 64
> +	     ? __memset_falkor
> +	     : __memset_generic));
>   
>   # undef memset
>   strong_alias (__libc_memset, memset);
> diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> index 33b87a8..203f839 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> @@ -35,6 +35,7 @@ static struct cpu_list cpu_list[] = {
>         {"thunderxt88",	 0x430F0A10},
>         {"thunderx2t99",   0x431F0AF0},
>         {"thunderx2t99p1", 0x420F5160},
> +      {"phecda",	 0x680F0000},
>         {"generic", 	 0x0}
>   };
>   
> diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
> index cde655b..eb35adf 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
> @@ -49,6 +49,9 @@
>   #define IS_FALKOR(midr) (MIDR_IMPLEMENTOR(midr) == 'Q'			      \
>                           && MIDR_PARTNUM(midr) == 0xc00)
>   
> +#define IS_PHECDA(midr) (MIDR_IMPLEMENTOR(midr) == 'h'			      \
> +                        && MIDR_PARTNUM(midr) == 0x000)
> +
>   struct cpu_features
>   {
>     uint64_t midr_el1;
> 

Re: [PATCH] aarch64: add HXT Phecda core memory operation ifuncs

[Hongbo Zhang]

On 11 June 2018 at 20:34, Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
> On 06/11/2018 03:16 PM, Hongbo Zhang wrote:
>>
>> Phecda is HXT semiconductor's CPU core, this patch adds memory operation
>> ifuncs for it: sharing the same optimized implementation with Qualcomm's
>> Falkor core.
>>
>> 2018-06-07  Minfeng Kang <minfeng.kang@hxt-semitech.com>
>>             Hongbo Zhang <hongbo.zhang@linaro.org>
>>
>>         * sysdeps/aarch64/multiarch/memcpy.c (libc_ifunc): reuse
>>         __memcpy_falkor for phecda core.
>>         * sysdeps/aarch64/multiarch/memmove.c (libc_ifunc): reuse
>>         __memmove_falkor for phecda core.
>>         * sysdeps/aarch64/multiarch/memset.c (libc_ifunc): reuse
>>         __memset_falkor for phecda core.
>>         * sysdeps/unix/sysv/linux/aarch64/cpu-features.c: add MIDR entry
>>         for phecda core.
>>         * sysdeps/unix/sysv/linux/aarch64/cpu-features.h (IS_PHECDA): add
>>         macro to identify phecda core.
>
>
> Thank you for the patch.  Your contributions are covered by the Linaro
> copyright assignment, but you have also attributed Minfeng Kang with part of
> this patch.  Can you (or Carlos/Joseph) please clarify the copyright
> assignment status for Minfeng?  I don't have access to the assignment file
> to confirm myself.
>
> Thanks,
> Siddhesh
>
Hi Siddhesh,
Thank you for the quick review.
It is my first time to upstream a glibc patch, rules seem a bit
different from kernel/uboot etc.
If it is up to me to clarify the copyright, how to do it, add a
Co-authored-by tag?

Thanks
>
>> ---
>>   sysdeps/aarch64/multiarch/memcpy.c             | 2 +-
>>   sysdeps/aarch64/multiarch/memmove.c            | 2 +-
>>   sysdeps/aarch64/multiarch/memset.c             | 7 ++++---
>>   sysdeps/unix/sysv/linux/aarch64/cpu-features.c | 1 +
>>   sysdeps/unix/sysv/linux/aarch64/cpu-features.h | 3 +++
>>   5 files changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/sysdeps/aarch64/multiarch/memcpy.c
>> b/sysdeps/aarch64/multiarch/memcpy.c
>> index b94c655..4a04a63 100644
>> --- a/sysdeps/aarch64/multiarch/memcpy.c
>> +++ b/sysdeps/aarch64/multiarch/memcpy.c
>> @@ -36,7 +36,7 @@ extern __typeof (__redirect_memcpy) __memcpy_falkor
>> attribute_hidden;
>>   libc_ifunc (__libc_memcpy,
>>               (IS_THUNDERX (midr)
>>              ? __memcpy_thunderx
>> -            : (IS_FALKOR (midr)
>> +            : (IS_FALKOR (midr) || IS_PHECDA (midr)
>>                 ? __memcpy_falkor
>>                 : (IS_THUNDERX2 (midr) || IS_THUNDERX2PA (midr)
>>                   ? __memcpy_thunderx2
>> diff --git a/sysdeps/aarch64/multiarch/memmove.c
>> b/sysdeps/aarch64/multiarch/memmove.c
>> index afd8dd2..e69d816 100644
>> --- a/sysdeps/aarch64/multiarch/memmove.c
>> +++ b/sysdeps/aarch64/multiarch/memmove.c
>> @@ -35,7 +35,7 @@ extern __typeof (__redirect_memmove) __memmove_falkor
>> attribute_hidden;
>>   libc_ifunc (__libc_memmove,
>>               (IS_THUNDERX (midr)
>>              ? __memmove_thunderx
>> -            : (IS_FALKOR (midr)
>> +            : (IS_FALKOR (midr) || IS_PHECDA (midr)
>>                 ? __memmove_falkor
>>                 : __memmove_generic)));
>>   diff --git a/sysdeps/aarch64/multiarch/memset.c
>> b/sysdeps/aarch64/multiarch/memset.c
>> index 6df93d5..d74ed3a 100644
>> --- a/sysdeps/aarch64/multiarch/memset.c
>> +++ b/sysdeps/aarch64/multiarch/memset.c
>> @@ -31,9 +31,10 @@ extern __typeof (__redirect_memset) __libc_memset;
>>   extern __typeof (__redirect_memset) __memset_falkor attribute_hidden;
>>   extern __typeof (__redirect_memset) __memset_generic attribute_hidden;
>>   -libc_ifunc (__libc_memset, (IS_FALKOR (midr) && zva_size == 64
>> -                           ? __memset_falkor
>> -                           : __memset_generic));
>> +libc_ifunc (__libc_memset,
>> +           ((IS_FALKOR (midr) || IS_PHECDA (midr)) && zva_size == 64
>> +            ? __memset_falkor
>> +            : __memset_generic));
>>     # undef memset
>>   strong_alias (__libc_memset, memset);
>> diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
>> b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
>> index 33b87a8..203f839 100644
>> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
>> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
>> @@ -35,6 +35,7 @@ static struct cpu_list cpu_list[] = {
>>         {"thunderxt88",  0x430F0A10},
>>         {"thunderx2t99",   0x431F0AF0},
>>         {"thunderx2t99p1", 0x420F5160},
>> +      {"phecda",        0x680F0000},
>>         {"generic",      0x0}
>>   };
>>   diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
>> b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
>> index cde655b..eb35adf 100644
>> --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
>> +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.h
>> @@ -49,6 +49,9 @@
>>   #define IS_FALKOR(midr) (MIDR_IMPLEMENTOR(midr) == 'Q'
>> \
>>                           && MIDR_PARTNUM(midr) == 0xc00)
>>   +#define IS_PHECDA(midr) (MIDR_IMPLEMENTOR(midr) == 'h'
>> \
>> +                        && MIDR_PARTNUM(midr) == 0x000)
>> +
>>   struct cpu_features
>>   {
>>     uint64_t midr_el1;
>>
>

Re: [PATCH] aarch64: add HXT Phecda core memory operation ifuncs

[Siddhesh Poyarekar]

On 06/12/2018 11:25 AM, Hongbo Zhang wrote:
> Thank you for the quick review.
> It is my first time to upstream a glibc patch, rules seem a bit
> different from kernel/uboot etc.
> If it is up to me to clarify the copyright, how to do it, add a
> Co-authored-by tag?

Yes, the rules are very different from kernel/uboot where the 
contributors continue to own copyright and there's typically only a 
developer sign-off to claim that copyright.  In glibc and gcc we have a 
concept of copyright assignment where we transfer ownership of code to 
the Free Software Foundation who we trust to defend our copyright in 
case of infringement.

The problem with the patch is not that of format; you have formatted the 
patch perfectly and would have been good to be committed, but we need a 
copyright assignment (i.e. an agreement that you're willing to transfer 
ownership of the copyright to this code to the FSF) from all authors of 
the patch.

Your contribution has an automatic assignment since you're a Linaro 
assignee, but that's not the case for the other author (AFAICT, again I 
don't have access to the copyrights file so I can't tell for sure, only 
the FSF stewards can) because of which we need to understand the scope 
of their contribution and their assignment status to decide on the 
future course of action on this patch.

Sorry for the trouble, but this is an unavoidable part of the patch 
review workflow.

Siddhesh

Re: [PATCH] aarch64: add HXT Phecda core memory operation ifuncs

[Hongbo Zhang]

On 12 June 2018 at 14:27, Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
> On 06/12/2018 11:25 AM, Hongbo Zhang wrote:
>>
>> Thank you for the quick review.
>> It is my first time to upstream a glibc patch, rules seem a bit
>> different from kernel/uboot etc.
>> If it is up to me to clarify the copyright, how to do it, add a
>> Co-authored-by tag?
>
>
> Yes, the rules are very different from kernel/uboot where the contributors
> continue to own copyright and there's typically only a developer sign-off to
> claim that copyright.  In glibc and gcc we have a concept of copyright
> assignment where we transfer ownership of code to the Free Software
> Foundation who we trust to defend our copyright in case of infringement.
>
> The problem with the patch is not that of format; you have formatted the
> patch perfectly and would have been good to be committed, but we need a
> copyright assignment (i.e. an agreement that you're willing to transfer
> ownership of the copyright to this code to the FSF) from all authors of the
> patch.
>
> Your contribution has an automatic assignment since you're a Linaro
> assignee, but that's not the case for the other author (AFAICT, again I
> don't have access to the copyrights file so I can't tell for sure, only the
> FSF stewards can) because of which we need to understand the scope of their
> contribution and their assignment status to decide on the future course of
> action on this patch.
>
> Sorry for the trouble, but this is an unavoidable part of the patch review
> workflow.
>
It is not a trouble, I learned the copyright assignment now, thank you.
But as to the exact steps, Shoud Minfeng answer questionnaire like this page?
https://www.fsf.org/licensing/assigning.html
He never did that before.


> Siddhesh

Re: [PATCH] aarch64: add HXT Phecda core memory operation ifuncs

[Florian Weimer]

On 06/12/2018 10:45 AM, Hongbo Zhang wrote:
> On 12 June 2018 at 14:27, Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>> On 06/12/2018 11:25 AM, Hongbo Zhang wrote:
>>>
>>> Thank you for the quick review.
>>> It is my first time to upstream a glibc patch, rules seem a bit
>>> different from kernel/uboot etc.
>>> If it is up to me to clarify the copyright, how to do it, add a
>>> Co-authored-by tag?
>>
>>
>> Yes, the rules are very different from kernel/uboot where the contributors
>> continue to own copyright and there's typically only a developer sign-off to
>> claim that copyright.  In glibc and gcc we have a concept of copyright
>> assignment where we transfer ownership of code to the Free Software
>> Foundation who we trust to defend our copyright in case of infringement.
>>
>> The problem with the patch is not that of format; you have formatted the
>> patch perfectly and would have been good to be committed, but we need a
>> copyright assignment (i.e. an agreement that you're willing to transfer
>> ownership of the copyright to this code to the FSF) from all authors of the
>> patch.
>>
>> Your contribution has an automatic assignment since you're a Linaro
>> assignee, but that's not the case for the other author (AFAICT, again I
>> don't have access to the copyrights file so I can't tell for sure, only the
>> FSF stewards can) because of which we need to understand the scope of their
>> contribution and their assignment status to decide on the future course of
>> action on this patch.
>>
>> Sorry for the trouble, but this is an unavoidable part of the patch review
>> workflow.
>>
> It is not a trouble, I learned the copyright assignment now, thank you.
> But as to the exact steps, Shoud Minfeng answer questionnaire like this page?
> https://www.fsf.org/licensing/assigning.html
> He never did that before.

The forms are linked from here:

<https://sourceware.org/glibc/wiki/Contribution%20checklist#FSF_copyright_Assignment>

For a company, usually someone who has authorization to negotiate and 
sign contracts has to do this, not individual patch authors.

In this case, I think we can still accept the patch because the number 
of lines changes is small (10 lines), but we'd need it for the next patch.

Thanks,
Florian

Re: [PATCH] aarch64: add HXT Phecda core memory operation ifuncs

[Hongbo Zhang]

On 12 June 2018 at 17:42, Florian Weimer <fweimer@redhat.com> wrote:
> On 06/12/2018 10:45 AM, Hongbo Zhang wrote:
>>
>> On 12 June 2018 at 14:27, Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>>>
>>> On 06/12/2018 11:25 AM, Hongbo Zhang wrote:
>>>>
>>>>
>>>> Thank you for the quick review.
>>>> It is my first time to upstream a glibc patch, rules seem a bit
>>>> different from kernel/uboot etc.
>>>> If it is up to me to clarify the copyright, how to do it, add a
>>>> Co-authored-by tag?
>>>
>>>
>>>
>>> Yes, the rules are very different from kernel/uboot where the
>>> contributors
>>> continue to own copyright and there's typically only a developer sign-off
>>> to
>>> claim that copyright.  In glibc and gcc we have a concept of copyright
>>> assignment where we transfer ownership of code to the Free Software
>>> Foundation who we trust to defend our copyright in case of infringement.
>>>
>>> The problem with the patch is not that of format; you have formatted the
>>> patch perfectly and would have been good to be committed, but we need a
>>> copyright assignment (i.e. an agreement that you're willing to transfer
>>> ownership of the copyright to this code to the FSF) from all authors of
>>> the
>>> patch.
>>>
>>> Your contribution has an automatic assignment since you're a Linaro
>>> assignee, but that's not the case for the other author (AFAICT, again I
>>> don't have access to the copyrights file so I can't tell for sure, only
>>> the
>>> FSF stewards can) because of which we need to understand the scope of
>>> their
>>> contribution and their assignment status to decide on the future course
>>> of
>>> action on this patch.
>>>
>>> Sorry for the trouble, but this is an unavoidable part of the patch
>>> review
>>> workflow.
>>>
>> It is not a trouble, I learned the copyright assignment now, thank you.
>> But as to the exact steps, Shoud Minfeng answer questionnaire like this
>> page?
>> https://www.fsf.org/licensing/assigning.html
>> He never did that before.
>
>
> The forms are linked from here:
>
> <https://sourceware.org/glibc/wiki/Contribution%20checklist#FSF_copyright_Assignment>
>
> For a company, usually someone who has authorization to negotiate and sign
> contracts has to do this, not individual patch authors.
>
> In this case, I think we can still accept the patch because the number of
> lines changes is small (10 lines), but we'd need it for the next patch.
>
Get it, thanks.

> Thanks,
> Florian

Re: [PATCH] aarch64: add HXT Phecda core memory operation ifuncs

[Siddhesh Poyarekar]

On 06/12/2018 03:12 PM, Florian Weimer wrote:
> For a company, usually someone who has authorization to negotiate and 
> sign contracts has to do this, not individual patch authors.
> 
> In this case, I think we can still accept the patch because the number 
> of lines changes is small (10 lines), but we'd need it for the next patch.

I am ambivalent about the legal significance since although the number 
of lines of the change is small it could be argued as being a 
significant feature addition given that it chooses optimal routines for 
a specific processor core and implicitly provides information about the 
Phecda microarchitecture.  As such, it is borderline newsworthy, 
speaking of which, maybe it needs a NEWS entry like so:

  * Improved string routine performance for the Aarch64 HXT Phecda core.

Siddhesh

Re: [PATCH] aarch64: add HXT Phecda core memory operation ifuncs

[Siddhesh Poyarekar]

On 06/12/2018 03:33 PM, Siddhesh Poyarekar wrote:
> I am ambivalent about the legal significance since although the number 
> of lines of the change is small it could be argued as being a 
> significant feature addition given that it chooses optimal routines for 
> a specific processor core and implicitly provides information about the 
> Phecda microarchitecture.  As such, it is borderline newsworthy, 
> speaking of which, maybe it needs a NEWS entry like so:
> 
>   * Improved string routine performance for the Aarch64 HXT Phecda core.

To be clear, I am perfectly OK with committing the patch for Hongbo if 
it is consensus that the LOC test is sufficient for legal significance.

Siddhesh

Re: [PATCH] aarch64: add HXT Phecda core memory operation ifuncs

[Florian Weimer]

On 06/12/2018 12:03 PM, Siddhesh Poyarekar wrote:
> On 06/12/2018 03:12 PM, Florian Weimer wrote:
>> For a company, usually someone who has authorization to negotiate and 
>> sign contracts has to do this, not individual patch authors.
>>
>> In this case, I think we can still accept the patch because the number 
>> of lines changes is small (10 lines), but we'd need it for the next 
>> patch.
> 
> I am ambivalent about the legal significance since although the number 
> of lines of the change is small it could be argued as being a 
> significant feature addition given that it chooses optimal routines for 
> a specific processor core and implicitly provides information about the 
> Phecda microarchitecture.  As such, it is borderline newsworthy, 
> speaking of which, maybe it needs a NEWS entry like so:
> 
>   * Improved string routine performance for the Aarch64 HXT Phecda core.

To be clear, I did not comment on the significance of the patch (legal 
or otherwise).  I just repeated the rules used by the FSF used to 
determine if copyright assignment

| A change of just a few lines (less than 15 or so) is not legally
| significant for copyright.

<https://www.gnu.org/prep/maintain/html_node/Legally-Significant.html>

I think we all agree that the size of the patch has little to do with 
how much work went into it and how much impact it will have on users. 8-)

Thanks,
Florian

Re: [PATCH] aarch64: add HXT Phecda core memory operation ifuncs

[Siddhesh Poyarekar]

On 06/12/2018 03:47 PM, Florian Weimer wrote:
> | A change of just a few lines (less than 15 or so) is not legally
> | significant for copyright.
> 
> <https://www.gnu.org/prep/maintain/html_node/Legally-Significant.html>
> 
> I think we all agree that the size of the patch has little to do with 
> how much work went into it and how much impact it will have on users. 8-)

:)

That line is pretty unambiguous, so I'll commit this patch for Hongbo 
and follow up offline with him to clear the assignment issue for future 
patches.

Thanks,
Siddhesh

Re: [PATCH] aarch64: add HXT Phecda core memory operation ifuncs

[Hongbo Zhang]

OK, thank you all for help.

On 12 June 2018 at 18:28, Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
> On 06/12/2018 03:47 PM, Florian Weimer wrote:
>>
>> | A change of just a few lines (less than 15 or so) is not legally
>> | significant for copyright.
>>
>> <https://www.gnu.org/prep/maintain/html_node/Legally-Significant.html>
>>
>> I think we all agree that the size of the patch has little to do with how
>> much work went into it and how much impact it will have on users. 8-)
>
>
> :)
>
> That line is pretty unambiguous, so I'll commit this patch for Hongbo and
> follow up offline with him to clear the assignment issue for future patches.
>
> Thanks,
> Siddhesh