Re: [PATCH v2] staging: mt7621-eth/ethtool.c: Correction of SPDX license identifier

Re: [PATCH v2] staging: mt7621-eth/ethtool.c: Correction of SPDX license identifier

[gregkh]

On Wed, Jan 30, 2019 at 02:48:27PM +0000, Carlos Henrique Lima Melara wrote:
> 	This patch fix the checkpatch.p1 warning:
> 
> 	WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
> 	+/*

Why did you not fix up the indentation that was already mentioned?

> 	It includes the SPDX expression for GPL-2.0 and corrects the comment format to suit the kernel's coding style.

Always properly line-wrap your comments.

> 
> Signed-off-by: Carlos (Charles) Henrique Lima Melara <charlesmelara@outlook.com>

This needs to match your "From:" line.

thanks,

greg k-h

Re: [PATCH v2] staging: mt7621-eth/ethtool.c: Correction of SPDX license identifier

[gregkh]

On Wed, Jan 30, 2019 at 08:01:13PM +0000, Carlos Henrique Lima Melara wrote:
> On 30/01/2019 13:29, gregkh@linuxfoundation.org wrote:
> > On Wed, Jan 30, 2019 at 02:48:27PM +0000, Carlos Henrique Lima Melara wrote:
> >> 	This patch fix the checkpatch.p1 warning:
> >>
> >> 	WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
> >> 	+/*
> >
> > Why did you not fix up the indentation that was already mentioned?
> 
> The Warning was about the missing SPDX License Identifier, so I added the 
> identifier according to the "license-rules.rst" for C source that is:

I mean the indentation of the text in your changelog, nothing to do with
the actual change in your patch below, sorry for the confusion.

> >   The SPDX license identifier is added in form of a comment.  The comment
> >   style depends on the file type::
> >
> >      C source:	// SPDX-License-Identifier: <SPDX License Expression>
> >      C header:	/* SPDX-License-Identifier: <SPDX License Expression> */
> >      ASM:	/* SPDX-License-Identifier: <SPDX License Expression> */
> >      scripts:	# SPDX-License-Identifier: <SPDX License Expression>
> >      .rst:	.. SPDX-License-Identifier: <SPDX License Expression>
> >      .dts{i}:	// SPDX-License-Identifier: <SPDX License Expression>
> 
> I think that was the correction needed, right?

Yes.

> Still have a question about the license comment in the "ethtooll.c". 
> 
> > /*   This program is free software; you can redistribute it and/or modify
> >  *   it under the terms of the GNU General Public License as published by
> >  *   the Free Software Foundation; version 2 of the License
> >  *
> >  *   This program is distributed in the hope that it will be useful,
> >  *   but WITHOUT ANY WARRANTY; without even the implied warranty of
> >  *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >  *   GNU General Public License for more details.
> >  *
> >  *   Copyright (C) 2009-2016 John Crispin <blogic@openwrt.org>
> >  *   Copyright (C) 2009-2016 Felix Fietkau <nbd@openwrt.org>
> >  *   Copyright (C) 2013-2016 Michael Lee <igvtee@gmail.com>
> >  */
> 
> It puts 3 spaces between the * and the text, should it be corrected to 
> only one space?

If you really want to, but it's not needed.

All of the "boilerplate" license text can also be removed after the SPDX
line is added, if you want to do that.  But leave the copyright lines,
that needs to stay.

thanks,

greg k-h

Re: [PATCH v2] staging: mt7621-eth/ethtool.c: Correction of SPDX license identifier

[Andrew Lunn]

On Wed, Jan 30, 2019 at 02:48:27PM +0000, Carlos Henrique Lima Melara wrote:
> 	This patch fix the checkpatch.p1 warning:
> 
> 	WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
> 	+/*
> 
> 	It includes the SPDX expression for GPL-2.0 and corrects the comment format to suit the kernel's coding style.
> 
> Signed-off-by: Carlos (Charles) Henrique Lima Melara <charlesmelara@outlook.com>
> ---
>  drivers/staging/mt7621-eth/ethtool.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/staging/mt7621-eth/ethtool.c b/drivers/staging/mt7621-eth/ethtool.c
> index 40a7d47be913..49d417de64c8 100644
> --- a/drivers/staging/mt7621-eth/ethtool.c
> +++ b/drivers/staging/mt7621-eth/ethtool.c
> @@ -1,15 +1,17 @@
> -/*   This program is free software; you can redistribute it and/or modify
> - *   it under the terms of the GNU General Public License as published by
> - *   the Free Software Foundation; version 2 of the License
> +// SPDX-License-Identifier: GPL-2.0

Hi Carlos

drivers/staging/mt7621-eth$ grep LICENSE *
gsw_mt7621.c:MODULE_LICENSE("GPL");
mtk_eth_soc.c:MODULE_LICENSE("GPL");

And include/linux/module.h 
says:

/*
 * The following license idents are currently accepted as indicating free
 * software modules
 *
 *      "GPL"                           [GNU Public License v2 or later]
 *      "GPL v2"                        [GNU Public License v2]

So the SPDX string probably does not match the MODULE_LICENSE.

   Andrew

Re: [PATCH v2] staging: mt7621-eth/ethtool.c: Correction of SPDX license identifier

[gregkh]

On Wed, Jan 30, 2019 at 05:38:17PM +0100, Andrew Lunn wrote:
> On Wed, Jan 30, 2019 at 02:48:27PM +0000, Carlos Henrique Lima Melara wrote:
> > 	This patch fix the checkpatch.p1 warning:
> > 
> > 	WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
> > 	+/*
> > 
> > 	It includes the SPDX expression for GPL-2.0 and corrects the comment format to suit the kernel's coding style.
> > 
> > Signed-off-by: Carlos (Charles) Henrique Lima Melara <charlesmelara@outlook.com>
> > ---
> >  drivers/staging/mt7621-eth/ethtool.c | 22 ++++++++++++----------
> >  1 file changed, 12 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/staging/mt7621-eth/ethtool.c b/drivers/staging/mt7621-eth/ethtool.c
> > index 40a7d47be913..49d417de64c8 100644
> > --- a/drivers/staging/mt7621-eth/ethtool.c
> > +++ b/drivers/staging/mt7621-eth/ethtool.c
> > @@ -1,15 +1,17 @@
> > -/*   This program is free software; you can redistribute it and/or modify
> > - *   it under the terms of the GNU General Public License as published by
> > - *   the Free Software Foundation; version 2 of the License
> > +// SPDX-License-Identifier: GPL-2.0
> 
> Hi Carlos
> 
> drivers/staging/mt7621-eth$ grep LICENSE *
> gsw_mt7621.c:MODULE_LICENSE("GPL");
> mtk_eth_soc.c:MODULE_LICENSE("GPL");
> 
> And include/linux/module.h 
> says:
> 
> /*
>  * The following license idents are currently accepted as indicating free
>  * software modules
>  *
>  *      "GPL"                           [GNU Public License v2 or later]
>  *      "GPL v2"                        [GNU Public License v2]
> 
> So the SPDX string probably does not match the MODULE_LICENSE.

No, ignore the MODULE_LICENSE marking for this, go by what the text at
the top of the file says. See the patch about all of this from Thomas on
lkml yesterday for why this is the case.

So the patch is fine.  If the changelog stuff ever gets fixed...

thanks,

greg k-h

Re: [PATCH v2] staging: mt7621-eth/ethtool.c: Correction of SPDX license identifier

[Andrew Lunn]

> See the patch about all of this from Thomas on lkml yesterday for
> why this is the case.

Hi Greg

Thanks for the info. I had not seen this, and i guess other have not
as well. So here is a link to the patch.

https://lkml.org/lkml/2019/1/28/1975

	Andrew