* [PATCH 2/9 v3] staging: spi: mt7621: Clean up excessive header usage
2019-02-01 10:17 [PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier Stefan Roese
@ 2019-02-01 10:17 ` Stefan Roese
2019-02-01 10:17 ` [PATCH 3/9 v3] staging: spi: mt7621: Add return code check on device_reset() Stefan Roese
` (7 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Stefan Roese @ 2019-02-01 10:17 UTC (permalink / raw)
To: linux-spi, devel
Cc: Greg Kroah-Hartman, Sankalp Negi, Mark Brown, John Crispin, NeilBrown
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 3/9 v3] staging: spi: mt7621: Add return code check on device_reset()
2019-02-01 10:17 [PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier Stefan Roese
2019-02-01 10:17 ` [PATCH 2/9 v3] staging: spi: mt7621: Clean up excessive header usage Stefan Roese
@ 2019-02-01 10:17 ` Stefan Roese
2019-02-01 10:17 ` [PATCH 4/9 v3] staging: spi: mt7621: Remove superfluous SPI_BPW_MASK definition Stefan Roese
` (6 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Stefan Roese @ 2019-02-01 10:17 UTC (permalink / raw)
To: linux-spi, devel
Cc: Greg Kroah-Hartman, Sankalp Negi, Mark Brown, John Crispin, NeilBrown
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 4/9 v3] staging: spi: mt7621: Remove superfluous SPI_BPW_MASK definition
2019-02-01 10:17 [PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier Stefan Roese
2019-02-01 10:17 ` [PATCH 2/9 v3] staging: spi: mt7621: Clean up excessive header usage Stefan Roese
2019-02-01 10:17 ` [PATCH 3/9 v3] staging: spi: mt7621: Add return code check on device_reset() Stefan Roese
@ 2019-02-01 10:17 ` Stefan Roese
2019-02-01 10:17 ` [PATCH 5/9 v3] staging: spi: mt7621: Minor cosmetic changes Stefan Roese
` (5 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Stefan Roese @ 2019-02-01 10:17 UTC (permalink / raw)
To: linux-spi, devel
Cc: Greg Kroah-Hartman, Sankalp Negi, Mark Brown, John Crispin, NeilBrown
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 5/9 v3] staging: spi: mt7621: Minor cosmetic changes
2019-02-01 10:17 [PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier Stefan Roese
` (2 preceding siblings ...)
2019-02-01 10:17 ` [PATCH 4/9 v3] staging: spi: mt7621: Remove superfluous SPI_BPW_MASK definition Stefan Roese
@ 2019-02-01 10:17 ` Stefan Roese
2019-02-01 10:17 ` [PATCH 6/9 v3] staging: spi: mt7621: Use recommended comment style Stefan Roese
` (4 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Stefan Roese @ 2019-02-01 10:17 UTC (permalink / raw)
To: linux-spi, devel
Cc: Greg Kroah-Hartman, Sankalp Negi, Mark Brown, John Crispin, NeilBrown
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 6/9 v3] staging: spi: mt7621: Use recommended comment style
2019-02-01 10:17 [PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier Stefan Roese
` (3 preceding siblings ...)
2019-02-01 10:17 ` [PATCH 5/9 v3] staging: spi: mt7621: Minor cosmetic changes Stefan Roese
@ 2019-02-01 10:17 ` Stefan Roese
2019-02-01 10:17 ` [PATCH 7/9 v3] staging: spi: mt7621: Sort register definitions Stefan Roese
` (3 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Stefan Roese @ 2019-02-01 10:17 UTC (permalink / raw)
To: linux-spi, devel
Cc: Greg Kroah-Hartman, Sankalp Negi, Mark Brown, John Crispin, NeilBrown
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 7/9 v3] staging: spi: mt7621: Sort register definitions
2019-02-01 10:17 [PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier Stefan Roese
` (4 preceding siblings ...)
2019-02-01 10:17 ` [PATCH 6/9 v3] staging: spi: mt7621: Use recommended comment style Stefan Roese
@ 2019-02-01 10:17 ` Stefan Roese
2019-02-01 10:17 ` [PATCH 8/9 v3] staging: spi: mt7621: Use macros instead of hardcoded values Stefan Roese
` (2 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Stefan Roese @ 2019-02-01 10:17 UTC (permalink / raw)
To: linux-spi, devel
Cc: Greg Kroah-Hartman, Sankalp Negi, Mark Brown, John Crispin, NeilBrown
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 8/9 v3] staging: spi: mt7621: Use macros instead of hardcoded values
2019-02-01 10:17 [PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier Stefan Roese
` (5 preceding siblings ...)
2019-02-01 10:17 ` [PATCH 7/9 v3] staging: spi: mt7621: Sort register definitions Stefan Roese
@ 2019-02-01 10:17 ` Stefan Roese
2019-02-01 10:17 ` [PATCH 9/9 v3] staging: spi: mt7621: Remove superfluous pre-declaration of struct mt7621_spi Stefan Roese
2019-02-01 13:03 ` [PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier Mark Brown
8 siblings, 0 replies; 15+ messages in thread
From: Stefan Roese @ 2019-02-01 10:17 UTC (permalink / raw)
To: linux-spi, devel
Cc: Greg Kroah-Hartman, Sankalp Negi, Mark Brown, John Crispin, NeilBrown
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 9/9 v3] staging: spi: mt7621: Remove superfluous pre-declaration of struct mt7621_spi
2019-02-01 10:17 [PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier Stefan Roese
` (6 preceding siblings ...)
2019-02-01 10:17 ` [PATCH 8/9 v3] staging: spi: mt7621: Use macros instead of hardcoded values Stefan Roese
@ 2019-02-01 10:17 ` Stefan Roese
2019-02-03 22:42 ` NeilBrown
2019-02-01 13:03 ` [PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier Mark Brown
8 siblings, 1 reply; 15+ messages in thread
From: Stefan Roese @ 2019-02-01 10:17 UTC (permalink / raw)
To: linux-spi, devel
Cc: Greg Kroah-Hartman, Sankalp Negi, Mark Brown, John Crispin, NeilBrown
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 9/9 v3] staging: spi: mt7621: Remove superfluous pre-declaration of struct mt7621_spi
2019-02-01 10:17 ` [PATCH 9/9 v3] staging: spi: mt7621: Remove superfluous pre-declaration of struct mt7621_spi Stefan Roese
@ 2019-02-03 22:42 ` NeilBrown
0 siblings, 0 replies; 15+ messages in thread
From: NeilBrown @ 2019-02-03 22:42 UTC (permalink / raw)
To: Stefan Roese, linux-spi, devel
Cc: Greg Kroah-Hartman, John Crispin, Mark Brown, Sankalp Negi
[-- 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier
2019-02-01 10:17 [PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier Stefan Roese
` (7 preceding siblings ...)
2019-02-01 10:17 ` [PATCH 9/9 v3] staging: spi: mt7621: Remove superfluous pre-declaration of struct mt7621_spi Stefan Roese
@ 2019-02-01 13:03 ` Mark Brown
2019-02-03 22:34 ` NeilBrown
8 siblings, 1 reply; 15+ messages in thread
From: Mark Brown @ 2019-02-01 13:03 UTC (permalink / raw)
To: Stefan Roese
Cc: devel, Greg Kroah-Hartman, John Crispin, linux-spi, Sankalp Negi,
NeilBrown
[-- 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
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier
2019-02-01 13:03 ` [PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier Mark Brown
@ 2019-02-03 22:34 ` NeilBrown
2019-02-04 8:53 ` Mark Brown
0 siblings, 1 reply; 15+ messages in thread
From: NeilBrown @ 2019-02-03 22:34 UTC (permalink / raw)
To: Mark Brown, Stefan Roese
Cc: devel, Greg Kroah-Hartman, linux-spi, Sankalp Negi, John Crispin
[-- 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier
2019-02-03 22:34 ` NeilBrown
@ 2019-02-04 8:53 ` Mark Brown
2019-02-04 23:09 ` NeilBrown
2019-02-05 6:29 ` Stefan Roese
0 siblings, 2 replies; 15+ messages in thread
From: Mark Brown @ 2019-02-04 8:53 UTC (permalink / raw)
To: NeilBrown
Cc: devel, Greg Kroah-Hartman, John Crispin, linux-spi, Sankalp Negi,
Stefan Roese
[-- 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier
2019-02-04 8:53 ` Mark Brown
@ 2019-02-04 23:09 ` NeilBrown
2019-02-05 6:29 ` Stefan Roese
1 sibling, 0 replies; 15+ messages in thread
From: NeilBrown @ 2019-02-04 23:09 UTC (permalink / raw)
To: Mark Brown
Cc: devel, Greg Kroah-Hartman, John Crispin, linux-spi, Sankalp Negi,
Stefan Roese
[-- 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
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 1/9 v3] staging: spi: mt7621: Switch to SPDX identifier
2019-02-04 8:53 ` Mark Brown
2019-02-04 23:09 ` NeilBrown
@ 2019-02-05 6:29 ` Stefan Roese
1 sibling, 0 replies; 15+ messages in thread
From: Stefan Roese @ 2019-02-05 6:29 UTC (permalink / raw)
To: Mark Brown, NeilBrown
Cc: devel, Greg Kroah-Hartman, linux-spi, Sankalp Negi, John Crispin
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
^ permalink raw reply [flat|nested] 15+ messages in thread