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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Sat, 13 Aug 2022, Julien Grall wrote:
> 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?

I used the information in COPYING at the top of the repository. I
realize that I should also have checked the comment header at the top
of each source file, especially in regards to GPLv2 vs, GPLv2 or later.
I'll do that in the next version.


> 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/*.

Excellent point. I realized there are a couple of style details to sort
out so I sent out a separate email about that.


> > 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.

I agree. Also I made a comment about this in the other larger thread to
make sure we are all on the same page about it.


> > 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.

Thanks


> >   #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.

[...]

> Same here about GPLv2+. Please go through the rest of the files to confirm the
> license.

The change was not intentional: this exercise should not result in any
licensing changes. Next time I'll make sure to check all the
copyright/license headers at the top of the files to make sure they
match the SPDX tag.

Thanks for spotting this.

Cheers,

Stefano