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.stabell...@amd.com> > > --- > > > > 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
