| Message ID | 20190201101715.3760-1-sr@denx.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9ad67a121637ae141ebaf04c1a27e60fdd91bab3 |
| Headers | show |
| Series | [1/9,v3] staging: spi: mt7621: Switch to SPDX identifier | expand |
On Fri, Feb 01, 2019 at 11:17:07AM +0100, Stefan Roese wrote: > @@ -1,3 +1,4 @@ > +// SPDX-License-Identifier: GPL-2.0 > /* > * spi-mt7621.c -- MediaTek MT7621 SPI controller driver > * Please convert the entire comment into a C++ comment so it looks more intentional.
On Fri, Feb 01 2019, Mark Brown wrote: > On Fri, Feb 01, 2019 at 11:17:07AM +0100, Stefan Roese wrote: > >> @@ -1,3 +1,4 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> /* >> * spi-mt7621.c -- MediaTek MT7621 SPI controller driver >> * > > Please convert the entire comment into a C++ comment so it looks more > intentional. Hi Mark, I don't understand what you mean by this, or possibly I do understand and strongly disagree. It is extremely common in the kernel for a file to start // SPDX-License-Identifier..... and to have that immediately followed by a comment lile: /* * ..... * .... This patch makes this file match much of the rest of the kernel. Why do you want something different? Thanks, NeilBrown
On Mon, Feb 04, 2019 at 09:34:56AM +1100, NeilBrown wrote: > It is extremely common in the kernel for a file to start > // SPDX-License-Identifier..... > and to have that immediately followed by a comment lile: > /* > * ..... > * .... Yes, there was a lot of automated conversion AFAICT (and a lot of confusion with all this stuff only being documented in random mailing list posts for a long time). > This patch makes this file match much of the rest of the kernel. Why > do you want something different? Like I said because it makes the comments look more like someone actually meant to add a C++ comment - it's what the rest of the subsystem is doing too.
On Mon, Feb 04 2019, Mark Brown wrote: > On Mon, Feb 04, 2019 at 09:34:56AM +1100, NeilBrown wrote: > >> It is extremely common in the kernel for a file to start >> // SPDX-License-Identifier..... > >> and to have that immediately followed by a comment lile: > >> /* >> * ..... >> * .... > > Yes, there was a lot of automated conversion AFAICT (and a lot of > confusion with all this stuff only being documented in random mailing > list posts for a long time). > >> This patch makes this file match much of the rest of the kernel. Why >> do you want something different? > > Like I said because it makes the comments look more like someone > actually meant to add a C++ comment - it's what the rest of the > subsystem is doing too. Ahh.. the argument "what the rest of the subsystem is doing" makes a lot more sense to me than "look more like someone actually mean to add a C++ comment", because I really don't understand how // SPDX-License-Identifier..... /* * old comment */ doesn't look like it was meant to be added. Looking around the kernel, the pattern of "// SPDX-..." followed by a "//" happens a lot in 99 arch/csky/ 122 arch/nds32/ 152 sound/soc/ 201 drivers/media/ 273 arch/arm/ (numbers are how many instances I found), and less so in 10 drivers/soc/ 11 drivers/soundwire/ 12 drivers/misc/ 12 drivers/pci/ 13 drivers/mfd/ 13 drivers/power/ 14 drivers/dma/ 14 drivers/input/ 14 scripts/coccinelle/ 16 drivers/leds/ 18 drivers/spi/ 25 arch/arm64/ 26 drivers/regulator/ 28 drivers/clk/ 29 drivers/net/ 36 tools/testing/ 52 drivers/pinctrl/ and much less else where. Not at all in fs/ or mm/ or net/ It would be nice to have some consistency. I would be in favour of avoiding // as much as possible. The "// SPDX-" lines were (presumably) added nearly automatically. I would prefer that if it were don't manually, it would always be /* * SPDX- .... * etc */ but I guess that horse has left the gate, though we do have that pattern in about 129 files. What a mess. Any way, if you make a case based on "that is what most other files in drivers/spi/ do", then I won't have any disagreement with that. Thanks, NeilBrown
On 04.02.19 09:53, Mark Brown wrote: > On Mon, Feb 04, 2019 at 09:34:56AM +1100, NeilBrown wrote: > >> It is extremely common in the kernel for a file to start >> // SPDX-License-Identifier..... > >> and to have that immediately followed by a comment lile: > >> /* >> * ..... >> * .... > > Yes, there was a lot of automated conversion AFAICT (and a lot of > confusion with all this stuff only being documented in random mailing > list posts for a long time). > >> This patch makes this file match much of the rest of the kernel. Why >> do you want something different? > > Like I said because it makes the comments look more like someone > actually meant to add a C++ comment - it's what the rest of the > subsystem is doing too. If nobody objects, I will do this change to C++ comments in the header with the move out of staging into drivers/spi. Thanks, Stefan
diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c b/drivers/staging/mt7621-spi/spi-mt7621.c index 513b6e79b985..c2f6f9ce52a2 100644 --- a/drivers/staging/mt7621-spi/spi-mt7621.c +++ b/drivers/staging/mt7621-spi/spi-mt7621.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 /* * spi-mt7621.c -- MediaTek MT7621 SPI controller driver * @@ -8,10 +9,6 @@ * Some parts are based on spi-orion.c: * Author: Shadi Ammouri <shadi@marvell.com> * Copyright (C) 2007-2008 Marvell Ltd. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. */ #include <linux/init.h>
Adopt the SPDX license identifier headers to ease license compliance management. Signed-off-by: Stefan Roese <sr@denx.de> Cc: Mark Brown <broonie@kernel.org> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: NeilBrown <neil@brown.name> Cc: Sankalp Negi <sankalpnegi2310@gmail.com> Cc: Chuanhong Guo <gch981213@gmail.com> Cc: John Crispin <john@phrozen.org> --- v3: - No change v2: - Changes are done to the driver in staging before moving it out of staging into drivers/spi drivers/staging/mt7621-spi/spi-mt7621.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)