Re: [PATCH] add SPDX to arch/arm/*.c

[Julien Grall <julien@xxxxxxx>]

Hi Stefano,

On 13/08/2022 01:59, Stefano Stabellini wrote:
> Add SPDX license information to all the *.c files under arch/arm.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxx>
> ---
>
> We need to start from somewhere and I thought arch/arm/*.c would be a
> good place to start.
Thanks for doing it. This will make easier to understand the license in 
each file. There are a couple of places below where the SDPX tag is 
incorrect. How did you figure out the which license to use?
Also, I think we should consider to add a section about SPDX in our 
coding style so new files are using it. So we don't end up with a mix in 
arch/arm/*.
> diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c
> index f03cd943c6..8115f89408 100644
> --- a/xen/arch/arm/alternative.c
> +++ b/xen/arch/arm/alternative.c
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0
Technically, this is a comment. So this should be /* ... */ to follow 
Xen coding style. Also...
>   /*
>    * alternative runtime patching
>    * inspired by the x86 version
... this comment contains information about the license. As you add the 
SPDX, the "long" version should be removed. This would also make easier 
to verify the SPDX you add match existing license.
> diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
> index ec81a45de9..7c986ecb18 100644
> --- a/xen/arch/arm/bootfdt.c
> +++ b/xen/arch/arm/bootfdt.c
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0
>   /*
>    * Early Device Tree
>    *
> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
> index ae649d16ef..887b5426c7 100644
> --- a/xen/arch/arm/cpuerrata.c
> +++ b/xen/arch/arm/cpuerrata.c
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0
This file had no explicit license. I had a look at the 'git log' and 
AFAICT this was either new code and came from Linux. So this looks fine 
to add GPLv2 here.
>   #include <xen/cpu.h>
>   #include <xen/cpumask.h>
>   #include <xen/init.h>
> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
> index 62d5e1770a..a6253cb57f 100644
> --- a/xen/arch/arm/cpufeature.c
> +++ b/xen/arch/arm/cpufeature.c
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0
>   /*
>    * Contains CPU feature definitions
>    *
> diff --git a/xen/arch/arm/decode.c b/xen/arch/arm/decode.c
> index f5f6562600..f586c3d781 100644
> --- a/xen/arch/arm/decode.c
> +++ b/xen/arch/arm/decode.c
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0
This tag doesn't match the license below. It is currently GPLv2+. I 
don't think you can change it without consulting the author. But if it 
is, then it should be mentioned in the commit message.
I remember we discussed in the past that some files were GPLv2+. But I 
can't remember what was the outcome (I can't find the thread). IIRC 
GPLv2+ is a lot more restrictive than GPLv2 and could prevent some 
companies to contribute.
[...]

> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 2cd481979c..1a2dac95a9 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0
Same here about GPLv2+. Please go through the rest of the files to 
confirm the license.
Cheers,

--
Julien Grall