[PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier

[PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier

[Stefan Roese]

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(-)

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

[PATCH 2/9 v3] staging: spi: mt7621: Clean up excessive header usage

[Stefan Roese]

This patch removes unnecessary header includes and sorts the remaining
ones alphabetically.

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:
- New patch, changes spilt into separate patches

 drivers/staging/mt7621-spi/spi-mt7621.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c b/drivers/staging/mt7621-spi/spi-mt7621.c
index c2f6f9ce52a2..440c3f77fde8 100644
--- a/drivers/staging/mt7621-spi/spi-mt7621.c
+++ b/drivers/staging/mt7621-spi/spi-mt7621.c
@@ -11,19 +11,13 @@
  *   Copyright (C) 2007-2008 Marvell Ltd.
  */
 
-#include <linux/init.h>
-#include <linux/module.h>
 #include <linux/clk.h>
-#include <linux/err.h>
 #include <linux/delay.h>
 #include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
 #include <linux/reset.h>
 #include <linux/spi/spi.h>
-#include <linux/of_device.h>
-#include <linux/platform_device.h>
-#include <linux/swab.h>
-
-#include <ralink_regs.h>
 
 #define SPI_BPW_MASK(bits) BIT((bits) - 1)
 
-- 
2.20.1

[PATCH 3/9 v3] staging: spi: mt7621: Add return code check on device_reset()

[Stefan Roese]

This patch adds a return code check on device_reset() and removes the
compile warning.

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:
- New patch, changes spilt into separate patches

 drivers/staging/mt7621-spi/spi-mt7621.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c b/drivers/staging/mt7621-spi/spi-mt7621.c
index 440c3f77fde8..ca4632740827 100644
--- a/drivers/staging/mt7621-spi/spi-mt7621.c
+++ b/drivers/staging/mt7621-spi/spi-mt7621.c
@@ -321,6 +321,7 @@ static int mt7621_spi_probe(struct platform_device *pdev)
 	int status = 0;
 	struct clk *clk;
 	struct mt7621_spi_ops *ops;
+	int ret;
 
 	match = of_match_device(mt7621_spi_match, &pdev->dev);
 	if (!match)
@@ -368,7 +369,11 @@ static int mt7621_spi_probe(struct platform_device *pdev)
 	rs->pending_write = 0;
 	dev_info(&pdev->dev, "sys_freq: %u\n", rs->sys_freq);
 
-	device_reset(&pdev->dev);
+	ret = device_reset(&pdev->dev);
+	if (ret) {
+		dev_err(&pdev->dev, "SPI reset failed!\n");
+		return ret;
+	}
 
 	mt7621_spi_reset(rs);
 
-- 
2.20.1

[PATCH 4/9 v3] staging: spi: mt7621: Remove superfluous SPI_BPW_MASK definition

[Stefan Roese]

This patch removes the SPI_BPW_MASK definition as this is already
available in include/linux/spi/spi.h.

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:
- New patch, changes spilt into separate patches

 drivers/staging/mt7621-spi/spi-mt7621.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c b/drivers/staging/mt7621-spi/spi-mt7621.c
index ca4632740827..fe9c863af1cc 100644
--- a/drivers/staging/mt7621-spi/spi-mt7621.c
+++ b/drivers/staging/mt7621-spi/spi-mt7621.c
@@ -19,8 +19,6 @@
 #include <linux/reset.h>
 #include <linux/spi/spi.h>
 
-#define SPI_BPW_MASK(bits) BIT((bits) - 1)
-
 #define DRIVER_NAME			"spi-mt7621"
 /* in usec */
 #define RALINK_SPI_WAIT_MAX_LOOP	2000
-- 
2.20.1

[PATCH 5/9 v3] staging: spi: mt7621: Minor cosmetic changes

[Stefan Roese]

Align  macro definitions and add some empty lines to make the code better
readable.

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:
- New patch, changes spilt into separate patches

 drivers/staging/mt7621-spi/spi-mt7621.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c b/drivers/staging/mt7621-spi/spi-mt7621.c
index fe9c863af1cc..e548f816ea9e 100644
--- a/drivers/staging/mt7621-spi/spi-mt7621.c
+++ b/drivers/staging/mt7621-spi/spi-mt7621.c
@@ -19,12 +19,13 @@
 #include <linux/reset.h>
 #include <linux/spi/spi.h>
 
-#define DRIVER_NAME			"spi-mt7621"
+#define DRIVER_NAME		"spi-mt7621"
+
 /* in usec */
-#define RALINK_SPI_WAIT_MAX_LOOP	2000
+#define RALINK_SPI_WAIT_MAX_LOOP 2000
 
 /* SPISTAT register bit field */
-#define SPISTAT_BUSY			BIT(0)
+#define SPISTAT_BUSY		BIT(0)
 
 #define MT7621_SPI_TRANS	0x00
 #define SPITRANS_BUSY		BIT(16)
@@ -186,6 +187,7 @@ static void mt7621_spi_read_half_duplex(struct mt7621_spi *rs,
 			*buf++ = val & 0xff;
 			val >>= 8;
 		}
+
 		rx_len -= i;
 	}
 }
@@ -279,6 +281,7 @@ static int mt7621_spi_transfer_one_message(struct spi_master *master,
 	mt7621_spi_flush(rs);
 
 	mt7621_spi_set_cs(spi, 0);
+
 msg_done:
 	m->status = status;
 	spi_finalize_current_message(master);
-- 
2.20.1

[PATCH 6/9 v3] staging: spi: mt7621: Use recommended comment style

[Stefan Roese]

This patch changes some comments to use the recommended multi-line
comment style.

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:
- New patch, changes spilt into separate patches

 drivers/staging/mt7621-spi/spi-mt7621.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c b/drivers/staging/mt7621-spi/spi-mt7621.c
index e548f816ea9e..00cff75ee7ac 100644
--- a/drivers/staging/mt7621-spi/spi-mt7621.c
+++ b/drivers/staging/mt7621-spi/spi-mt7621.c
@@ -123,10 +123,10 @@ static int mt7621_spi_prepare(struct spi_device *spi, unsigned int speed)
 	if (spi->mode & SPI_LSB_FIRST)
 		reg |= MT7621_LSB_FIRST;
 
-	/* This SPI controller seems to be tested on SPI flash only
-	 * and some bits are swizzled under other SPI modes probably
-	 * due to incorrect wiring inside the silicon. Only mode 0
-	 * works correctly.
+	/*
+	 * This SPI controller seems to be tested on SPI flash only and some
+	 * bits are swizzled under other SPI modes probably due to incorrect
+	 * wiring inside the silicon. Only mode 0 works correctly.
 	 */
 	reg &= ~(MT7621_CPHA | MT7621_CPOL);
 
@@ -155,9 +155,10 @@ static inline int mt7621_spi_wait_till_ready(struct mt7621_spi *rs)
 static void mt7621_spi_read_half_duplex(struct mt7621_spi *rs,
 					int rx_len, u8 *buf)
 {
-	/* Combine with any pending write, and perform one or
-	 * more half-duplex transactions reading 'len' bytes.
-	 * Data to be written is already in MT7621_SPI_DATA*
+	/*
+	 * Combine with any pending write, and perform one or more half-duplex
+	 * transactions reading 'len' bytes. Data to be written is already in
+	 * MT7621_SPI_DATA.
 	 */
 	int tx_len = rs->pending_write;
 
-- 
2.20.1

[PATCH 7/9 v3] staging: spi: mt7621: Sort register definitions

[Stefan Roese]

This patch sorts the SPI register definitions by increasing addresses.

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:
- New patch, changes spilt into separate patches

 drivers/staging/mt7621-spi/spi-mt7621.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c b/drivers/staging/mt7621-spi/spi-mt7621.c
index 00cff75ee7ac..89586a895320 100644
--- a/drivers/staging/mt7621-spi/spi-mt7621.c
+++ b/drivers/staging/mt7621-spi/spi-mt7621.c
@@ -36,9 +36,9 @@
 #define SPI_CTL_TX_RX_CNT_MASK	0xff
 #define SPI_CTL_START		BIT(8)
 
-#define MT7621_SPI_POLAR	0x38
 #define MT7621_SPI_MASTER	0x28
 #define MT7621_SPI_MOREBUF	0x2c
+#define MT7621_SPI_POLAR	0x38
 #define MT7621_SPI_SPACE	0x3c
 
 #define MT7621_CPHA		BIT(5)
-- 
2.20.1

[PATCH 8/9 v3] staging: spi: mt7621: Use macros instead of hardcoded values

[Stefan Roese]

This patch uses macros instead of hardcoded values for the SPI_MASTER
register access in mt7621_spi_reset() and mt7621_spi_prepare().

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:
- New patch, changes spilt into separate patches

 drivers/staging/mt7621-spi/spi-mt7621.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c b/drivers/staging/mt7621-spi/spi-mt7621.c
index 89586a895320..d6385220b796 100644
--- a/drivers/staging/mt7621-spi/spi-mt7621.c
+++ b/drivers/staging/mt7621-spi/spi-mt7621.c
@@ -37,6 +37,12 @@
 #define SPI_CTL_START		BIT(8)
 
 #define MT7621_SPI_MASTER	0x28
+#define MASTER_MORE_BUFMODE	BIT(2)
+#define MASTER_FULL_DUPLEX	BIT(10)
+#define MASTER_RS_CLK_SEL	GENMASK(27, 16)
+#define MASTER_RS_CLK_SEL_SHIFT	16
+#define MASTER_RS_SLAVE_SEL	GENMASK(31, 29)
+
 #define MT7621_SPI_MOREBUF	0x2c
 #define MT7621_SPI_POLAR	0x38
 #define MT7621_SPI_SPACE	0x3c
@@ -77,9 +83,13 @@ static void mt7621_spi_reset(struct mt7621_spi *rs)
 {
 	u32 master = mt7621_spi_read(rs, MT7621_SPI_MASTER);
 
-	master |= 7 << 29;
-	master |= 1 << 2;
-	master &= ~(1 << 10);
+	/*
+	 * Select SPI device 7, enable "more buffer mode" and disable
+	 * full-duplex (only half-duplex really works on this chip
+	 * reliably)
+	 */
+	master |= MASTER_RS_SLAVE_SEL | MASTER_MORE_BUFMODE;
+	master &= ~MASTER_FULL_DUPLEX;
 
 	mt7621_spi_write(rs, MT7621_SPI_MASTER, master);
 	rs->pending_write = 0;
@@ -115,8 +125,8 @@ static int mt7621_spi_prepare(struct spi_device *spi, unsigned int speed)
 		rate = 2;
 
 	reg = mt7621_spi_read(rs, MT7621_SPI_MASTER);
-	reg &= ~(0xfff << 16);
-	reg |= (rate - 2) << 16;
+	reg &= ~MASTER_RS_CLK_SEL;
+	reg |= (rate - 2) << MASTER_RS_CLK_SEL_SHIFT;
 	rs->speed = speed;
 
 	reg &= ~MT7621_LSB_FIRST;
-- 
2.20.1

[PATCH 9/9 v3] staging: spi: mt7621: Remove superfluous pre-declaration of struct mt7621_spi

[Stefan Roese]

This patch removes the superfluous pre-declaration of struct mt7621_spi.

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:
- New patch, changes spilt into separate patches

 drivers/staging/mt7621-spi/spi-mt7621.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c b/drivers/staging/mt7621-spi/spi-mt7621.c
index d6385220b796..167d0f09823b 100644
--- a/drivers/staging/mt7621-spi/spi-mt7621.c
+++ b/drivers/staging/mt7621-spi/spi-mt7621.c
@@ -51,8 +51,6 @@
 #define MT7621_CPOL		BIT(4)
 #define MT7621_LSB_FIRST	BIT(3)
 
-struct mt7621_spi;
-
 struct mt7621_spi {
 	struct spi_master	*master;
 	void __iomem		*base;
-- 
2.20.1

Re: [PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 276 bytes --]

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.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier

[NeilBrown]

[-- Attachment #1.1: Type: text/plain, Size: 737 bytes --]

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 9/9 v3] staging: spi: mt7621: Remove superfluous pre-declaration of struct mt7621_spi

[NeilBrown]

[-- Attachment #1.1: Type: text/plain, Size: 1182 bytes --]

On Fri, Feb 01 2019, Stefan Roese wrote:

> This patch removes the superfluous pre-declaration of struct mt7621_spi.
>
> 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:
> - New patch, changes spilt into separate patches
>
>  drivers/staging/mt7621-spi/spi-mt7621.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c b/drivers/staging/mt7621-spi/spi-mt7621.c
> index d6385220b796..167d0f09823b 100644
> --- a/drivers/staging/mt7621-spi/spi-mt7621.c
> +++ b/drivers/staging/mt7621-spi/spi-mt7621.c
> @@ -51,8 +51,6 @@
>  #define MT7621_CPOL		BIT(4)
>  #define MT7621_LSB_FIRST	BIT(3)
>  
> -struct mt7621_spi;
> -
>  struct mt7621_spi {
>  	struct spi_master	*master;
>  	void __iomem		*base;
> -- 
> 2.20.1

Thanks,
this and all previous patches in the series
  Review-by: NeilBrown <neil@brown.name>

(compile-tested too :-)

NeilBrown

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 678 bytes --]

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.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier

[NeilBrown]

[-- Attachment #1.1: Type: text/plain, Size: 2291 bytes --]

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier

[Stefan Roese]

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