[PATCH 0/6] fix line over 80 char & coding style in wilc_spi.c

[PATCH 0/6] fix line over 80 char & coding style in wilc_spi.c

[Ajay Singh]

This patch series contains changes to remove "fix line over 80 char"
issues found by checkpatch.pl script.
Also, few changes code changes are done to follow Linux coding style.

Ajay Singh (6):
  staging: wilc1000: modified code comments as per linux coding style
  staging: wilc1000: removed the unnecessary commented code
  staging: wilc1000: fix line over 80 characters in spi_cmd_complete()
  staging: wilc1000: fix line over 80 characters in wilc_spi_init()
  staging: wilc1000: fix line over 80 characters in wilc_spi_read_int()
  staging: wilc1000: fix line over 80 chars in wilc_spi_clear_int_ext()

 drivers/staging/wilc1000/wilc_spi.c | 551 ++++++++++++++++++------------------
 1 file changed, 278 insertions(+), 273 deletions(-)

-- 
2.7.4

[PATCH 1/6] staging: wilc1000: modified code comments as per linux coding style

[Ajay Singh]

Cleanup patch to follow the comments style as per the Linux coding
style.

Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
---
 drivers/staging/wilc1000/wilc_spi.c | 151 ++++++++++++++++++++----------------
 1 file changed, 82 insertions(+), 69 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_spi.c b/drivers/staging/wilc1000/wilc_spi.c
index 8f71a60..5d0de4e 100644
--- a/drivers/staging/wilc1000/wilc_spi.c
+++ b/drivers/staging/wilc1000/wilc_spi.c
@@ -422,9 +422,9 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
 		return N_FAIL;
 	}
 
-	/**
+	/*
 	 * Command/Control response
-	 **/
+	 */
 	if (cmd == CMD_RESET ||
 	    cmd == CMD_TERMINATE ||
 	    cmd == CMD_REPEAT) {
@@ -443,9 +443,9 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
 		return N_FAIL;
 	}
 
-	/**
+	/*
 	 * State response
-	 **/
+	 */
 	rsp = rb[rix++];
 	if (rsp != 0x00) {
 		dev_err(&spi->dev, "Failed cmd state response state (%02x)\n",
@@ -458,12 +458,15 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
 		int retry;
 		/* u16 crc1, crc2; */
 		u8 crc[2];
-		/**
+		/*
 		 * Data Respnose header
-		 **/
+		 */
 		retry = 100;
 		do {
-			/* ensure there is room in buffer later to read data and crc */
+			/*
+			 * ensure there is room in buffer later
+			 * to read data and crc
+			 */
 			if (rix < len2) {
 				rsp = rb[rix++];
 			} else {
@@ -481,9 +484,9 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
 		}
 
 		if (cmd == CMD_INTERNAL_READ || cmd == CMD_SINGLE_READ) {
-			/**
+			/*
 			 * Read bytes
-			 **/
+			 */
 			if ((rix + 3) < len2) {
 				b[0] = rb[rix++];
 				b[1] = rb[rix++];
@@ -496,9 +499,9 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
 			}
 
 			if (!g_spi.crc_off) {
-				/**
+				/*
 				 * Read Crc
-				 **/
+				 */
 				if ((rix + 1) < len2) {
 					crc[0] = rb[rix++];
 					crc[1] = rb[rix++];
@@ -524,18 +527,18 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
 				else
 					nbytes = DATA_PKT_SZ - ix;
 
-				/**
+				/*
 				 * Read bytes
-				 **/
+				 */
 				if (wilc_spi_rx(wilc, &b[ix], nbytes)) {
 					dev_err(&spi->dev, "Failed data block read, bus error...\n");
 					result = N_FAIL;
 					goto _error_;
 				}
 
-				/**
+				/*
 				 * Read Crc
-				 **/
+				 */
 				if (!g_spi.crc_off) {
 					if (wilc_spi_rx(wilc, crc, 2)) {
 						dev_err(&spi->dev, "Failed data block crc read, bus error...\n");
@@ -548,7 +551,10 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
 				sz -= nbytes;
 			}
 
-			/*  if any data in left unread, then read the rest using normal DMA code.*/
+			/*
+			 * if any data in left unread,
+			 * then read the rest using normal DMA code.
+			 */
 			while (sz > 0) {
 				int nbytes;
 
@@ -557,14 +563,14 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
 				else
 					nbytes = DATA_PKT_SZ;
 
-				/**
+				/*
 				 * read data response only on the next DMA cycles not
 				 * the first DMA since data response header is already
 				 * handled above for the first DMA.
-				 **/
-				/**
+				 */
+				/*
 				 * Data Respnose header
-				 **/
+				 */
 				retry = 10;
 				do {
 					if (wilc_spi_rx(wilc, &rsp, 1)) {
@@ -579,18 +585,18 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
 				if (result == N_FAIL)
 					break;
 
-				/**
+				/*
 				 * Read bytes
-				 **/
+				 */
 				if (wilc_spi_rx(wilc, &b[ix], nbytes)) {
 					dev_err(&spi->dev, "Failed data block read, bus error...\n");
 					result = N_FAIL;
 					break;
 				}
 
-				/**
+				/*
 				 * Read Crc
-				 **/
+				 */
 				if (!g_spi.crc_off) {
 					if (wilc_spi_rx(wilc, crc, 2)) {
 						dev_err(&spi->dev, "Failed data block crc read, bus error...\n");
@@ -616,9 +622,9 @@ static int spi_data_write(struct wilc *wilc, u8 *b, u32 sz)
 	u8 cmd, order, crc[2] = {0};
 	/* u8 rsp; */
 
-	/**
-	 *      Data
-	 **/
+	/*
+	 * Data
+	 */
 	ix = 0;
 	do {
 		if (sz <= DATA_PKT_SZ)
@@ -626,9 +632,9 @@ static int spi_data_write(struct wilc *wilc, u8 *b, u32 sz)
 		else
 			nbytes = DATA_PKT_SZ;
 
-		/**
-		 *      Write command
-		 **/
+		/*
+		 * Write command
+		 */
 		cmd = 0xf0;
 		if (ix == 0) {
 			if (sz <= DATA_PKT_SZ)
@@ -650,9 +656,9 @@ static int spi_data_write(struct wilc *wilc, u8 *b, u32 sz)
 			break;
 		}
 
-		/**
-		 *      Write data
-		 **/
+		/*
+		 * Write data
+		 */
 		if (wilc_spi_tx(wilc, &b[ix], nbytes)) {
 			dev_err(&spi->dev,
 				"Failed data block write, bus error...\n");
@@ -660,9 +666,9 @@ static int spi_data_write(struct wilc *wilc, u8 *b, u32 sz)
 			break;
 		}
 
-		/**
-		 *      Write Crc
-		 **/
+		/*
+		 * Write Crc
+		 */
 		if (!g_spi.crc_off) {
 			if (wilc_spi_tx(wilc, crc, 2)) {
 				dev_err(&spi->dev, "Failed data block crc write, bus error...\n");
@@ -671,9 +677,9 @@ static int spi_data_write(struct wilc *wilc, u8 *b, u32 sz)
 			}
 		}
 
-		/**
-		 *      No need to wait for response
-		 **/
+		/*
+		 * No need to wait for response
+		 */
 		ix += nbytes;
 		sz -= nbytes;
 	} while (sz);
@@ -733,7 +739,7 @@ static int wilc_spi_write_reg(struct wilc *wilc, u32 addr, u32 data)
 
 	data = cpu_to_le32(data);
 	if (addr < 0x30) {
-		/* Clockless register*/
+		/* Clockless register */
 		cmd = CMD_INTERNAL_WRITE;
 		clockless = 1;
 	}
@@ -751,9 +757,9 @@ static int wilc_spi_write(struct wilc *wilc, u32 addr, u8 *buf, u32 size)
 	int result;
 	u8 cmd = CMD_DMA_EXT_WRITE;
 
-	/**
-	 *      has to be greated than 4
-	 **/
+	/*
+	 * has to be greated than 4
+	 */
 	if (size <= 4)
 		return 0;
 
@@ -764,9 +770,9 @@ static int wilc_spi_write(struct wilc *wilc, u32 addr, u8 *buf, u32 size)
 		return 0;
 	}
 
-	/**
-	 *      Data
-	 **/
+	/*
+	 * Data
+	 */
 	result = spi_data_write(wilc, buf, size);
 	if (result != N_OK)
 		dev_err(&spi->dev, "Failed block data write...\n");
@@ -783,7 +789,7 @@ static int wilc_spi_read_reg(struct wilc *wilc, u32 addr, u32 *data)
 
 	if (addr < 0x30) {
 		/* dev_err(&spi->dev, "***** read addr %d\n\n", addr); */
-		/* Clockless register*/
+		/* Clockless register */
 		cmd = CMD_INTERNAL_READ;
 		clockless = 1;
 	}
@@ -825,9 +831,9 @@ static int wilc_spi_read(struct wilc *wilc, u32 addr, u8 *buf, u32 size)
 
 static int _wilc_spi_deinit(struct wilc *wilc)
 {
-	/**
-	 *      TODO:
-	 **/
+	/*
+	 * TODO:
+	 */
 	return 1;
 }
 
@@ -849,15 +855,19 @@ static int wilc_spi_init(struct wilc *wilc, bool resume)
 
 	memset(&g_spi, 0, sizeof(struct wilc_spi));
 
-	/**
-	 *      configure protocol
-	 **/
+	/*
+	 * configure protocol
+	 */
 	g_spi.crc_off = 0;
 
-	/* TODO: We can remove the CRC trials if there is a definite way to reset */
+	/*
+	 * TODO: We can remove the CRC trials if there is a definite
+	 * way to reset
+	 */
 	/* the SPI to it's initial value. */
 	if (!spi_internal_read(wilc, WILC_SPI_PROTOCOL_OFFSET, &reg)) {
-		/* Read failed. Try with CRC off. This might happen when module
+		/*
+		 * Read failed. Try with CRC off. This might happen when module
 		 * is removed but chip isn't reset
 		 */
 		g_spi.crc_off = 1;
@@ -870,7 +880,7 @@ static int wilc_spi_init(struct wilc *wilc, bool resume)
 		}
 	}
 	if (g_spi.crc_off == 0)	{
-		reg &= ~0xc;    /* disable crc checking */
+		reg &= ~0xc; /* disable crc checking */
 		reg &= ~0x70;
 		reg |= (0x5 << 4);
 		if (!spi_internal_write(wilc, WILC_SPI_PROTOCOL_OFFSET, reg)) {
@@ -880,9 +890,9 @@ static int wilc_spi_init(struct wilc *wilc, bool resume)
 		g_spi.crc_off = 1;
 	}
 
-	/**
-	 *      make sure can read back chip id correctly
-	 **/
+	/*
+	 * make sure can read back chip id correctly
+	 */
 	if (!wilc_spi_read_reg(wilc, 0x1000, &chipid)) {
 		dev_err(&spi->dev, "Fail cmd read chip id...\n");
 		return 0;
@@ -994,7 +1004,10 @@ static int wilc_spi_clear_int_ext(struct wilc *wilc, u32 val)
 
 			ret = 1;
 			for (i = 0; i < g_spi.nint; i++) {
-				/* No matter what you write 1 or 0, it will clear interrupt. */
+				/*
+				 * No matter what you write 1 or 0,
+				 * it will clear interrupt.
+				 */
 				if (flags & 1)
 					ret = wilc_spi_write_reg(wilc, 0x10c8 + i * 4, 1);
 				if (!ret)
@@ -1036,9 +1049,9 @@ static int wilc_spi_clear_int_ext(struct wilc *wilc, u32 val)
 			}
 
 			if ((val & EN_VMM) == EN_VMM) {
-				/**
-				 *      enable vmm transfer.
-				 **/
+				/*
+				 * enable vmm transfer.
+				 */
 				ret = wilc_spi_write_reg(wilc,
 							 WILC_VMM_CORE_CTL, 1);
 				if (!ret) {
@@ -1065,9 +1078,9 @@ static int wilc_spi_sync_ext(struct wilc *wilc, int nint)
 
 	g_spi.nint = nint;
 
-	/**
-	 *      interrupt pin mux select
-	 **/
+	/*
+	 * interrupt pin mux select
+	 */
 	ret = wilc_spi_read_reg(wilc, WILC_PIN_MUX_0, &reg);
 	if (!ret) {
 		dev_err(&spi->dev, "Failed read reg (%08x)...\n",
@@ -1082,9 +1095,9 @@ static int wilc_spi_sync_ext(struct wilc *wilc, int nint)
 		return 0;
 	}
 
-	/**
-	 *      interrupt enable
-	 **/
+	/*
+	 * interrupt enable
+	 */
 	ret = wilc_spi_read_reg(wilc, WILC_INTR_ENABLE, &reg);
 	if (!ret) {
 		dev_err(&spi->dev, "Failed read reg (%08x)...\n",
-- 
2.7.4

[PATCH 2/6] staging: wilc1000: removed the unnecessary commented code

[Ajay Singh]

Cleanup patch to remove the unused commented code.

Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
---
 drivers/staging/wilc1000/wilc_spi.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_spi.c b/drivers/staging/wilc1000/wilc_spi.c
index 5d0de4e..66b6aea 100644
--- a/drivers/staging/wilc1000/wilc_spi.c
+++ b/drivers/staging/wilc1000/wilc_spi.c
@@ -431,10 +431,7 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
 		rix++;         /* skip 1 byte */
 	}
 
-	/* do { */
 	rsp = rb[rix++];
-	/*	if(rsp == cmd) break; */
-	/* } while(&rptr[1] <= &rb[len2]); */
 
 	if (rsp != cmd) {
 		dev_err(&spi->dev,
@@ -456,7 +453,6 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
 	if (cmd == CMD_INTERNAL_READ || cmd == CMD_SINGLE_READ ||
 	    cmd == CMD_DMA_READ || cmd == CMD_DMA_EXT_READ) {
 		int retry;
-		/* u16 crc1, crc2; */
 		u8 crc[2];
 		/*
 		 * Data Respnose header
@@ -620,7 +616,6 @@ static int spi_data_write(struct wilc *wilc, u8 *b, u32 sz)
 	int ix, nbytes;
 	int result = 1;
 	u8 cmd, order, crc[2] = {0};
-	/* u8 rsp; */
 
 	/*
 	 * Data
@@ -897,7 +892,6 @@ static int wilc_spi_init(struct wilc *wilc, bool resume)
 		dev_err(&spi->dev, "Fail cmd read chip id...\n");
 		return 0;
 	}
-	/* dev_err(&spi->dev, "chipid (%08x)\n", chipid); */
 
 	g_spi.has_thrpt_enh = 1;
 
-- 
2.7.4

[PATCH 3/6] staging: wilc1000: fix line over 80 characters in spi_cmd_complete()

[Ajay Singh]

Refactor spi_cmd_complete() to fix the line over 80 char issues reported
by checkpatch.pl script.

Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
---
 drivers/staging/wilc1000/wilc_spi.c | 250 ++++++++++++++++++------------------
 1 file changed, 124 insertions(+), 126 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_spi.c b/drivers/staging/wilc1000/wilc_spi.c
index 66b6aea..30a9a61 100644
--- a/drivers/staging/wilc1000/wilc_spi.c
+++ b/drivers/staging/wilc1000/wilc_spi.c
@@ -287,17 +287,19 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
 	u8 rsp;
 	int len = 0;
 	int result = N_OK;
+	int retry;
+	u8 crc[2];
 
 	wb[0] = cmd;
 	switch (cmd) {
-	case CMD_SINGLE_READ:                           /* single word (4 bytes) read */
+	case CMD_SINGLE_READ: /* single word (4 bytes) read */
 		wb[1] = (u8)(adr >> 16);
 		wb[2] = (u8)(adr >> 8);
 		wb[3] = (u8)adr;
 		len = 5;
 		break;
 
-	case CMD_INTERNAL_READ:                 /* internal register read */
+	case CMD_INTERNAL_READ: /* internal register read */
 		wb[1] = (u8)(adr >> 8);
 		if (clockless == 1)
 			wb[1] |= BIT(7);
@@ -306,29 +308,29 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
 		len = 5;
 		break;
 
-	case CMD_TERMINATE:                                     /* termination */
+	case CMD_TERMINATE:
 		wb[1] = 0x00;
 		wb[2] = 0x00;
 		wb[3] = 0x00;
 		len = 5;
 		break;
 
-	case CMD_REPEAT:                                                /* repeat */
+	case CMD_REPEAT:
 		wb[1] = 0x00;
 		wb[2] = 0x00;
 		wb[3] = 0x00;
 		len = 5;
 		break;
 
-	case CMD_RESET:                                                 /* reset */
+	case CMD_RESET:
 		wb[1] = 0xff;
 		wb[2] = 0xff;
 		wb[3] = 0xff;
 		len = 5;
 		break;
 
-	case CMD_DMA_WRITE:                                     /* dma write */
-	case CMD_DMA_READ:                                      /* dma read */
+	case CMD_DMA_WRITE: /* dma write */
+	case CMD_DMA_READ:  /* dma read */
 		wb[1] = (u8)(adr >> 16);
 		wb[2] = (u8)(adr >> 8);
 		wb[3] = (u8)adr;
@@ -337,8 +339,8 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
 		len = 7;
 		break;
 
-	case CMD_DMA_EXT_WRITE:         /* dma extended write */
-	case CMD_DMA_EXT_READ:                  /* dma extended read */
+	case CMD_DMA_EXT_WRITE: /* dma extended write */
+	case CMD_DMA_EXT_READ:  /* dma extended read */
 		wb[1] = (u8)(adr >> 16);
 		wb[2] = (u8)(adr >> 8);
 		wb[3] = (u8)adr;
@@ -348,7 +350,7 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
 		len = 8;
 		break;
 
-	case CMD_INTERNAL_WRITE:                /* internal register write */
+	case CMD_INTERNAL_WRITE: /* internal register write */
 		wb[1] = (u8)(adr >> 8);
 		if (clockless == 1)
 			wb[1] |= BIT(7);
@@ -360,7 +362,7 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
 		len = 8;
 		break;
 
-	case CMD_SINGLE_WRITE:                  /* single word write */
+	case CMD_SINGLE_WRITE: /* single word write */
 		wb[1] = (u8)(adr >> 16);
 		wb[2] = (u8)(adr >> 8);
 		wb[3] = (u8)(adr);
@@ -395,13 +397,12 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
 	    cmd == CMD_REPEAT) {
 		len2 = len + (NUM_SKIP_BYTES + NUM_RSP_BYTES + NUM_DUMMY_BYTES);
 	} else if (cmd == CMD_INTERNAL_READ || cmd == CMD_SINGLE_READ) {
-		if (!g_spi.crc_off) {
-			len2 = len + (NUM_RSP_BYTES + NUM_DATA_HDR_BYTES + NUM_DATA_BYTES
-				      + NUM_CRC_BYTES + NUM_DUMMY_BYTES);
-		} else {
-			len2 = len + (NUM_RSP_BYTES + NUM_DATA_HDR_BYTES + NUM_DATA_BYTES
-				      + NUM_DUMMY_BYTES);
-		}
+		int tmp = NUM_RSP_BYTES + NUM_DATA_HDR_BYTES + NUM_DATA_BYTES
+			+ NUM_DUMMY_BYTES;
+		if (!g_spi.crc_off)
+			len2 = len + tmp + NUM_CRC_BYTES;
+		else
+			len2 = len + tmp;
 	} else {
 		len2 = len + (NUM_RSP_BYTES + NUM_DUMMY_BYTES);
 	}
@@ -425,11 +426,8 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
 	/*
 	 * Command/Control response
 	 */
-	if (cmd == CMD_RESET ||
-	    cmd == CMD_TERMINATE ||
-	    cmd == CMD_REPEAT) {
-		rix++;         /* skip 1 byte */
-	}
+	if (cmd == CMD_RESET || cmd == CMD_TERMINATE || cmd == CMD_REPEAT)
+		rix++; /* skip 1 byte */
 
 	rsp = rb[rix++];
 
@@ -452,8 +450,6 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
 
 	if (cmd == CMD_INTERNAL_READ || cmd == CMD_SINGLE_READ ||
 	    cmd == CMD_DMA_READ || cmd == CMD_DMA_EXT_READ) {
-		int retry;
-		u8 crc[2];
 		/*
 		 * Data Respnose header
 		 */
@@ -478,132 +474,134 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
 				"Error, data read response (%02x)\n", rsp);
 			return N_RESET;
 		}
+	}
+
+	if (cmd == CMD_INTERNAL_READ || cmd == CMD_SINGLE_READ) {
+		/*
+		 * Read bytes
+		 */
+		if ((rix + 3) < len2) {
+			b[0] = rb[rix++];
+			b[1] = rb[rix++];
+			b[2] = rb[rix++];
+			b[3] = rb[rix++];
+		} else {
+			dev_err(&spi->dev,
+				"buffer overrun when reading data.\n");
+			return N_FAIL;
+		}
 
-		if (cmd == CMD_INTERNAL_READ || cmd == CMD_SINGLE_READ) {
+		if (!g_spi.crc_off) {
 			/*
-			 * Read bytes
+			 * Read Crc
 			 */
-			if ((rix + 3) < len2) {
-				b[0] = rb[rix++];
-				b[1] = rb[rix++];
-				b[2] = rb[rix++];
-				b[3] = rb[rix++];
+			if ((rix + 1) < len2) {
+				crc[0] = rb[rix++];
+				crc[1] = rb[rix++];
 			} else {
 				dev_err(&spi->dev,
-					"buffer overrun when reading data.\n");
+					"buffer overrun when reading crc.\n");
 				return N_FAIL;
 			}
+		}
+	} else if ((cmd == CMD_DMA_READ) || (cmd == CMD_DMA_EXT_READ)) {
+		int ix;
 
-			if (!g_spi.crc_off) {
-				/*
-				 * Read Crc
-				 */
-				if ((rix + 1) < len2) {
-					crc[0] = rb[rix++];
-					crc[1] = rb[rix++];
-				} else {
-					dev_err(&spi->dev, "buffer overrun when reading crc.\n");
-					return N_FAIL;
-				}
-			}
-		} else if ((cmd == CMD_DMA_READ) || (cmd == CMD_DMA_EXT_READ)) {
-			int ix;
-
-			/* some data may be read in response to dummy bytes. */
-			for (ix = 0; (rix < len2) && (ix < sz); )
-				b[ix++] = rb[rix++];
-
-			sz -= ix;
-
-			if (sz > 0) {
-				int nbytes;
+		/* some data may be read in response to dummy bytes. */
+		for (ix = 0; (rix < len2) && (ix < sz); )
+			b[ix++] = rb[rix++];
 
-				if (sz <= (DATA_PKT_SZ - ix))
-					nbytes = sz;
-				else
-					nbytes = DATA_PKT_SZ - ix;
+		sz -= ix;
 
-				/*
-				 * Read bytes
-				 */
-				if (wilc_spi_rx(wilc, &b[ix], nbytes)) {
-					dev_err(&spi->dev, "Failed data block read, bus error...\n");
-					result = N_FAIL;
-					goto _error_;
-				}
+		if (sz > 0) {
+			int nbytes;
 
-				/*
-				 * Read Crc
-				 */
-				if (!g_spi.crc_off) {
-					if (wilc_spi_rx(wilc, crc, 2)) {
-						dev_err(&spi->dev, "Failed data block crc read, bus error...\n");
-						result = N_FAIL;
-						goto _error_;
-					}
-				}
+			if (sz <= (DATA_PKT_SZ - ix))
+				nbytes = sz;
+			else
+				nbytes = DATA_PKT_SZ - ix;
 
-				ix += nbytes;
-				sz -= nbytes;
+			/*
+			 * Read bytes
+			 */
+			if (wilc_spi_rx(wilc, &b[ix], nbytes)) {
+				dev_err(&spi->dev,
+					"Failed block read, bus err\n");
+				result = N_FAIL;
+				goto _error_;
 			}
 
 			/*
-			 * if any data in left unread,
-			 * then read the rest using normal DMA code.
+			 * Read Crc
 			 */
-			while (sz > 0) {
-				int nbytes;
+			if (!g_spi.crc_off && wilc_spi_rx(wilc, crc, 2)) {
+				dev_err(&spi->dev,
+					"Failed block crc read, bus err\n");
+				result = N_FAIL;
+				goto _error_;
+			}
 
-				if (sz <= DATA_PKT_SZ)
-					nbytes = sz;
-				else
-					nbytes = DATA_PKT_SZ;
+			ix += nbytes;
+			sz -= nbytes;
+		}
 
-				/*
-				 * read data response only on the next DMA cycles not
-				 * the first DMA since data response header is already
-				 * handled above for the first DMA.
-				 */
-				/*
-				 * Data Respnose header
-				 */
-				retry = 10;
-				do {
-					if (wilc_spi_rx(wilc, &rsp, 1)) {
-						dev_err(&spi->dev, "Failed data response read, bus error...\n");
-						result = N_FAIL;
-						break;
-					}
-					if (((rsp >> 4) & 0xf) == 0xf)
-						break;
-				} while (retry--);
-
-				if (result == N_FAIL)
-					break;
+		/*
+		 * if any data in left unread,
+		 * then read the rest using normal DMA code.
+		 */
+		while (sz > 0) {
+			int nbytes;
 
-				/*
-				 * Read bytes
-				 */
-				if (wilc_spi_rx(wilc, &b[ix], nbytes)) {
-					dev_err(&spi->dev, "Failed data block read, bus error...\n");
+			if (sz <= DATA_PKT_SZ)
+				nbytes = sz;
+			else
+				nbytes = DATA_PKT_SZ;
+
+			/*
+			 * read data response only on the next DMA cycles not
+			 * the first DMA since data response header is already
+			 * handled above for the first DMA.
+			 */
+			/*
+			 * Data Respnose header
+			 */
+			retry = 10;
+			do {
+				if (wilc_spi_rx(wilc, &rsp, 1)) {
+					dev_err(&spi->dev,
+						"Failed resp read, bus err\n");
 					result = N_FAIL;
 					break;
 				}
+				if (((rsp >> 4) & 0xf) == 0xf)
+					break;
+			} while (retry--);
 
-				/*
-				 * Read Crc
-				 */
-				if (!g_spi.crc_off) {
-					if (wilc_spi_rx(wilc, crc, 2)) {
-						dev_err(&spi->dev, "Failed data block crc read, bus error...\n");
-						result = N_FAIL;
-						break;
-					}
-				}
+			if (result == N_FAIL)
+				break;
 
-				ix += nbytes;
-				sz -= nbytes;
+			/*
+			 * Read bytes
+			 */
+			if (wilc_spi_rx(wilc, &b[ix], nbytes)) {
+				dev_err(&spi->dev,
+					"Failed block read, bus err\n");
+				result = N_FAIL;
+				break;
 			}
+
+			/*
+			 * Read Crc
+			 */
+			if (!g_spi.crc_off && wilc_spi_rx(wilc, crc, 2)) {
+				dev_err(&spi->dev,
+					"Failed block crc read, bus err\n");
+				result = N_FAIL;
+				break;
+			}
+
+			ix += nbytes;
+			sz -= nbytes;
 		}
 	}
 _error_:
-- 
2.7.4

[PATCH 4/6] staging: wilc1000: fix line over 80 characters in wilc_spi_init()

[Ajay Singh]

Modified wilc_spi_init() to fix the line over 80 char issues reported
by checkpatch.pl script.
To overcome the checkpatch.pl reported issue modified debug logs and
comments used in wilc_spi_init().

Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
---
 drivers/staging/wilc1000/wilc_spi.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_spi.c b/drivers/staging/wilc1000/wilc_spi.c
index 30a9a61..fddc0db 100644
--- a/drivers/staging/wilc1000/wilc_spi.c
+++ b/drivers/staging/wilc1000/wilc_spi.c
@@ -835,7 +835,6 @@ static int wilc_spi_init(struct wilc *wilc, bool resume)
 	struct spi_device *spi = to_spi_device(wilc->dev);
 	u32 reg;
 	u32 chipid;
-
 	static int isinit;
 
 	if (isinit) {
@@ -864,20 +863,25 @@ static int wilc_spi_init(struct wilc *wilc, bool resume)
 		 * is removed but chip isn't reset
 		 */
 		g_spi.crc_off = 1;
-		dev_err(&spi->dev, "Failed internal read protocol with CRC on, retrying with CRC off...\n");
+		dev_err(&spi->dev,
+			"Failed read with CRC on, retrying with CRC off\n");
 		if (!spi_internal_read(wilc, WILC_SPI_PROTOCOL_OFFSET, &reg)) {
-			/* Reaad failed with both CRC on and off, something went bad */
-			dev_err(&spi->dev,
-				"Failed internal read protocol...\n");
+			/*
+			 * Read failed with both CRC on and off,
+			 * something went bad
+			 */
+			dev_err(&spi->dev, "Failed internal read protocol\n");
 			return 0;
 		}
 	}
-	if (g_spi.crc_off == 0)	{
+	if (g_spi.crc_off == 0) {
 		reg &= ~0xc; /* disable crc checking */
 		reg &= ~0x70;
 		reg |= (0x5 << 4);
 		if (!spi_internal_write(wilc, WILC_SPI_PROTOCOL_OFFSET, reg)) {
-			dev_err(&spi->dev, "[wilc spi %d]: Failed internal write protocol reg...\n", __LINE__);
+			dev_err(&spi->dev,
+				"[wilc spi %d]: Failed internal write reg\n",
+				__LINE__);
 			return 0;
 		}
 		g_spi.crc_off = 1;
-- 
2.7.4

[PATCH 5/6] staging: wilc1000: fix line over 80 characters in wilc_spi_read_int()

[Ajay Singh]

Refactor wilc_spi_read_int() to fix the line over 80 char issues reported
by checkpatch.pl script.

Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
---
 drivers/staging/wilc1000/wilc_spi.c | 57 +++++++++++++++++++------------------
 1 file changed, 29 insertions(+), 28 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_spi.c b/drivers/staging/wilc1000/wilc_spi.c
index fddc0db..7c58beb8 100644
--- a/drivers/staging/wilc1000/wilc_spi.c
+++ b/drivers/staging/wilc1000/wilc_spi.c
@@ -939,45 +939,46 @@ static int wilc_spi_read_int(struct wilc *wilc, u32 *int_status)
 	int happened, j;
 	u32 unknown_mask;
 	u32 irq_flags;
+	int k = IRG_FLAGS_OFFSET + 5;
 
 	if (g_spi.has_thrpt_enh) {
 		ret = spi_internal_read(wilc, 0xe840 - WILC_SPI_REG_BASE,
 					int_status);
-	} else {
-		ret = wilc_spi_read_reg(wilc, WILC_VMM_TO_HOST_SIZE,
-					&byte_cnt);
-		if (!ret) {
-			dev_err(&spi->dev,
-				"Failed read WILC_VMM_TO_HOST_SIZE ...\n");
-			goto _fail_;
-		}
-		tmp = (byte_cnt >> 2) & IRQ_DMA_WD_CNT_MASK;
+		return ret;
+	}
+	ret = wilc_spi_read_reg(wilc, WILC_VMM_TO_HOST_SIZE, &byte_cnt);
+	if (!ret) {
+		dev_err(&spi->dev,
+			"Failed read WILC_VMM_TO_HOST_SIZE ...\n");
+		goto _fail_;
+	}
+	tmp = (byte_cnt >> 2) & IRQ_DMA_WD_CNT_MASK;
 
-		j = 0;
-		do {
-			happened = 0;
+	j = 0;
+	do {
+		happened = 0;
 
-			wilc_spi_read_reg(wilc, 0x1a90, &irq_flags);
-			tmp |= ((irq_flags >> 27) << IRG_FLAGS_OFFSET);
+		wilc_spi_read_reg(wilc, 0x1a90, &irq_flags);
+		tmp |= ((irq_flags >> 27) << IRG_FLAGS_OFFSET);
 
-			if (g_spi.nint > 5) {
-				wilc_spi_read_reg(wilc, 0x1a94,
-						  &irq_flags);
-				tmp |= (((irq_flags >> 0) & 0x7) << (IRG_FLAGS_OFFSET + 5));
-			}
+		if (g_spi.nint > 5) {
+			wilc_spi_read_reg(wilc, 0x1a94, &irq_flags);
+			tmp |= (((irq_flags >> 0) & 0x7) << k);
+		}
 
-			unknown_mask = ~((1ul << g_spi.nint) - 1);
+		unknown_mask = ~((1ul << g_spi.nint) - 1);
 
-			if ((tmp >> IRG_FLAGS_OFFSET) & unknown_mask) {
-				dev_err(&spi->dev, "Unexpected interrupt (2): j=%d, tmp=%x, mask=%x\n", j, tmp, unknown_mask);
-					happened = 1;
-			}
+		if ((tmp >> IRG_FLAGS_OFFSET) & unknown_mask) {
+			dev_err(&spi->dev,
+				"Unexpected interrupt(2):j=%d,tmp=%x,mask=%x\n",
+				j, tmp, unknown_mask);
+				happened = 1;
+		}
 
-			j++;
-		} while (happened);
+		j++;
+	} while (happened);
 
-		*int_status = tmp;
-	}
+	*int_status = tmp;
 
 _fail_:
 	return ret;
-- 
2.7.4

[PATCH 6/6] staging: wilc1000: fix line over 80 chars in wilc_spi_clear_int_ext()

[Ajay Singh]

Refactor wilc_spi_clear_int_ext() to fix the "line over 80 char" issue
reported by checkpatch.pl script.

Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
---
 drivers/staging/wilc1000/wilc_spi.c | 113 +++++++++++++++++-------------------
 1 file changed, 54 insertions(+), 59 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_spi.c b/drivers/staging/wilc1000/wilc_spi.c
index 7c58beb8..6b392c9 100644
--- a/drivers/staging/wilc1000/wilc_spi.c
+++ b/drivers/staging/wilc1000/wilc_spi.c
@@ -988,74 +988,69 @@ static int wilc_spi_clear_int_ext(struct wilc *wilc, u32 val)
 {
 	struct spi_device *spi = to_spi_device(wilc->dev);
 	int ret;
+	u32 flags;
+	u32 tbl_ctl;
 
 	if (g_spi.has_thrpt_enh) {
 		ret = spi_internal_write(wilc, 0xe844 - WILC_SPI_REG_BASE,
 					 val);
-	} else {
-		u32 flags;
-
-		flags = val & (BIT(MAX_NUM_INT) - 1);
-		if (flags) {
-			int i;
-
-			ret = 1;
-			for (i = 0; i < g_spi.nint; i++) {
-				/*
-				 * No matter what you write 1 or 0,
-				 * it will clear interrupt.
-				 */
-				if (flags & 1)
-					ret = wilc_spi_write_reg(wilc, 0x10c8 + i * 4, 1);
-				if (!ret)
-					break;
-				flags >>= 1;
-			}
-			if (!ret) {
+		return ret;
+	}
+
+	flags = val & (BIT(MAX_NUM_INT) - 1);
+	if (flags) {
+		int i;
+
+		ret = 1;
+		for (i = 0; i < g_spi.nint; i++) {
+			/*
+			 * No matter what you write 1 or 0,
+			 * it will clear interrupt.
+			 */
+			if (flags & 1)
+				ret = wilc_spi_write_reg(wilc,
+							 0x10c8 + i * 4, 1);
+			if (!ret)
+				break;
+			flags >>= 1;
+		}
+		if (!ret) {
+			dev_err(&spi->dev,
+				"Failed wilc_spi_write_reg, set reg %x ...\n",
+				0x10c8 + i * 4);
+			goto _fail_;
+		}
+		for (i = g_spi.nint; i < MAX_NUM_INT; i++) {
+			if (flags & 1)
 				dev_err(&spi->dev,
-					"Failed wilc_spi_write_reg, set reg %x ...\n",
-					0x10c8 + i * 4);
-				goto _fail_;
-			}
-			for (i = g_spi.nint; i < MAX_NUM_INT; i++) {
-				if (flags & 1)
-					dev_err(&spi->dev,
-						"Unexpected interrupt cleared %d...\n",
-						i);
-				flags >>= 1;
-			}
+					"Unexpected interrupt cleared %d...\n",
+					i);
+			flags >>= 1;
 		}
+	}
 
-		{
-			u32 tbl_ctl;
-
-			tbl_ctl = 0;
-			/* select VMM table 0 */
-			if ((val & SEL_VMM_TBL0) == SEL_VMM_TBL0)
-				tbl_ctl |= BIT(0);
-			/* select VMM table 1 */
-			if ((val & SEL_VMM_TBL1) == SEL_VMM_TBL1)
-				tbl_ctl |= BIT(1);
+	tbl_ctl = 0;
+	/* select VMM table 0 */
+	if ((val & SEL_VMM_TBL0) == SEL_VMM_TBL0)
+		tbl_ctl |= BIT(0);
+	/* select VMM table 1 */
+	if ((val & SEL_VMM_TBL1) == SEL_VMM_TBL1)
+		tbl_ctl |= BIT(1);
 
-			ret = wilc_spi_write_reg(wilc, WILC_VMM_TBL_CTL,
-						 tbl_ctl);
-			if (!ret) {
-				dev_err(&spi->dev,
-					"fail write reg vmm_tbl_ctl...\n");
-				goto _fail_;
-			}
+	ret = wilc_spi_write_reg(wilc, WILC_VMM_TBL_CTL, tbl_ctl);
+	if (!ret) {
+		dev_err(&spi->dev, "fail write reg vmm_tbl_ctl...\n");
+		goto _fail_;
+	}
 
-			if ((val & EN_VMM) == EN_VMM) {
-				/*
-				 * enable vmm transfer.
-				 */
-				ret = wilc_spi_write_reg(wilc,
-							 WILC_VMM_CORE_CTL, 1);
-				if (!ret) {
-					dev_err(&spi->dev, "fail write reg vmm_core_ctl...\n");
-					goto _fail_;
-				}
-			}
+	if ((val & EN_VMM) == EN_VMM) {
+		/*
+		 * enable vmm transfer.
+		 */
+		ret = wilc_spi_write_reg(wilc, WILC_VMM_CORE_CTL, 1);
+		if (!ret) {
+			dev_err(&spi->dev, "fail write reg vmm_core_ctl...\n");
+			goto _fail_;
 		}
 	}
 _fail_:
-- 
2.7.4

Re: [PATCH 4/6] staging: wilc1000: fix line over 80 characters in wilc_spi_init()

[Greg KH]

On Wed, Feb 14, 2018 at 04:40:13PM +0530, Ajay Singh wrote:
> Modified wilc_spi_init() to fix the line over 80 char issues reported
> by checkpatch.pl script.
> To overcome the checkpatch.pl reported issue modified debug logs and
> comments used in wilc_spi_init().
> 
> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
> ---
>  drivers/staging/wilc1000/wilc_spi.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/wilc1000/wilc_spi.c b/drivers/staging/wilc1000/wilc_spi.c
> index 30a9a61..fddc0db 100644
> --- a/drivers/staging/wilc1000/wilc_spi.c
> +++ b/drivers/staging/wilc1000/wilc_spi.c
> @@ -835,7 +835,6 @@ static int wilc_spi_init(struct wilc *wilc, bool resume)
>  	struct spi_device *spi = to_spi_device(wilc->dev);
>  	u32 reg;
>  	u32 chipid;
> -
>  	static int isinit;

You also deleted this line :(

Re: [PATCH 1/6] staging: wilc1000: modified code comments as per linux coding style

[Claudiu Beznea]

Hi Ajay,

Even I cannot locate something in Documentation/CodingStyle related to single
line comment, I prefer to use:
/* comment */

instead of:
/*
 * comment
 */

Do as you wish!

Thank you,
Claudiu

On 14.02.2018 13:10, Ajay Singh wrote:
> diff --git a/drivers/staging/wilc1000/wilc_spi.c b/drivers/staging/wilc1000/wilc_spi.c
> index 8f71a60..5d0de4e 100644
> --- a/drivers/staging/wilc1000/wilc_spi.c
> +++ b/drivers/staging/wilc1000/wilc_spi.c
> @@ -422,9 +422,9 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
>  		return N_FAIL;
>  	}
>  
> -	/**
> +	/*
>  	 * Command/Control response
> -	 **/
> +	 */
>  	if (cmd == CMD_RESET ||
>  	    cmd == CMD_TERMINATE ||
>  	    cmd == CMD_REPEAT) {
> @@ -443,9 +443,9 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
>  		return N_FAIL;
>  	}
>  
> -	/**
> +	/*
>  	 * State response
> -	 **/
> +	 */
>  	rsp = rb[rix++];
>  	if (rsp != 0x00) {
>  		dev_err(&spi->dev, "Failed cmd state response state (%02x)\n",
> @@ -458,12 +458,15 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
>  		int retry;
>  		/* u16 crc1, crc2; */
>  		u8 crc[2];
> -		/**
> +		/*
>  		 * Data Respnose header
> -		 **/
> +		 */
>  		retry = 100;
>  		do {
> -			/* ensure there is room in buffer later to read data and crc */
> +			/*
> +			 * ensure there is room in buffer later
> +			 * to read data and crc
> +			 */
>  			if (rix < len2) {
>  				rsp = rb[rix++];
>  			} else {
> @@ -481,9 +484,9 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
>  		}
>  
>  		if (cmd == CMD_INTERNAL_READ || cmd == CMD_SINGLE_READ) {
> -			/**
> +			/*
>  			 * Read bytes
> -			 **/
> +			 */
>  			if ((rix + 3) < len2) {
>  				b[0] = rb[rix++];
>  				b[1] = rb[rix++];
> @@ -496,9 +499,9 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
>  			}
>  
>  			if (!g_spi.crc_off) {
> -				/**
> +				/*
>  				 * Read Crc
> -				 **/
> +				 */
>  				if ((rix + 1) < len2) {
>  					crc[0] = rb[rix++];
>  					crc[1] = rb[rix++];
> @@ -524,18 +527,18 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
>  				else
>  					nbytes = DATA_PKT_SZ - ix;
>  
> -				/**
> +				/*
>  				 * Read bytes
> -				 **/
> +				 */
>  				if (wilc_spi_rx(wilc, &b[ix], nbytes)) {
>  					dev_err(&spi->dev, "Failed data block read, bus error...\n");
>  					result = N_FAIL;
>  					goto _error_;
>  				}
>  
> -				/**
> +				/*
>  				 * Read Crc
> -				 **/
> +				 */
>  				if (!g_spi.crc_off) {
>  					if (wilc_spi_rx(wilc, crc, 2)) {
>  						dev_err(&spi->dev, "Failed data block crc read, bus error...\n");
> @@ -548,7 +551,10 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
>  				sz -= nbytes;
>  			}
>  
> -			/*  if any data in left unread, then read the rest using normal DMA code.*/
> +			/*
> +			 * if any data in left unread,
> +			 * then read the rest using normal DMA code.
> +			 */
>  			while (sz > 0) {
>  				int nbytes;
>  
> @@ -557,14 +563,14 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
>  				else
>  					nbytes = DATA_PKT_SZ;
>  
> -				/**
> +				/*
>  				 * read data response only on the next DMA cycles not
>  				 * the first DMA since data response header is already
>  				 * handled above for the first DMA.
> -				 **/
> -				/**
> +				 */
> +				/*
>  				 * Data Respnose header
> -				 **/
> +				 */
>  				retry = 10;
>  				do {
>  					if (wilc_spi_rx(wilc, &rsp, 1)) {
> @@ -579,18 +585,18 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
>  				if (result == N_FAIL)
>  					break;
>  
> -				/**
> +				/*
>  				 * Read bytes
> -				 **/
> +				 */
>  				if (wilc_spi_rx(wilc, &b[ix], nbytes)) {
>  					dev_err(&spi->dev, "Failed data block read, bus error...\n");
>  					result = N_FAIL;
>  					break;
>  				}
>  
> -				/**
> +				/*
>  				 * Read Crc
> -				 **/
> +				 */
>  				if (!g_spi.crc_off) {
>  					if (wilc_spi_rx(wilc, crc, 2)) {
>  						dev_err(&spi->dev, "Failed data block crc read, bus error...\n");
> @@ -616,9 +622,9 @@ static int spi_data_write(struct wilc *wilc, u8 *b, u32 sz)
>  	u8 cmd, order, crc[2] = {0};
>  	/* u8 rsp; */
>  
> -	/**
> -	 *      Data
> -	 **/
> +	/*
> +	 * Data
> +	 */
>  	ix = 0;
>  	do {
>  		if (sz <= DATA_PKT_SZ)
> @@ -626,9 +632,9 @@ static int spi_data_write(struct wilc *wilc, u8 *b, u32 sz)
>  		else
>  			nbytes = DATA_PKT_SZ;
>  
> -		/**
> -		 *      Write command
> -		 **/
> +		/*
> +		 * Write command
> +		 */
>  		cmd = 0xf0;
>  		if (ix == 0) {
>  			if (sz <= DATA_PKT_SZ)
> @@ -650,9 +656,9 @@ static int spi_data_write(struct wilc *wilc, u8 *b, u32 sz)
>  			break;
>  		}
>  
> -		/**
> -		 *      Write data
> -		 **/
> +		/*
> +		 * Write data
> +		 */
>  		if (wilc_spi_tx(wilc, &b[ix], nbytes)) {
>  			dev_err(&spi->dev,
>  				"Failed data block write, bus error...\n");
> @@ -660,9 +666,9 @@ static int spi_data_write(struct wilc *wilc, u8 *b, u32 sz)
>  			break;
>  		}
>  
> -		/**
> -		 *      Write Crc
> -		 **/
> +		/*
> +		 * Write Crc
> +		 */
>  		if (!g_spi.crc_off) {
>  			if (wilc_spi_tx(wilc, crc, 2)) {
>  				dev_err(&spi->dev, "Failed data block crc write, bus error...\n");
> @@ -671,9 +677,9 @@ static int spi_data_write(struct wilc *wilc, u8 *b, u32 sz)
>  			}
>  		}
>  
> -		/**
> -		 *      No need to wait for response
> -		 **/
> +		/*
> +		 * No need to wait for response
> +		 */
>  		ix += nbytes;
>  		sz -= nbytes;
>  	} while (sz);
> @@ -733,7 +739,7 @@ static int wilc_spi_write_reg(struct wilc *wilc, u32 addr, u32 data)
>  
>  	data = cpu_to_le32(data);
>  	if (addr < 0x30) {
> -		/* Clockless register*/
> +		/* Clockless register */
>  		cmd = CMD_INTERNAL_WRITE;
>  		clockless = 1;
>  	}
> @@ -751,9 +757,9 @@ static int wilc_spi_write(struct wilc *wilc, u32 addr, u8 *buf, u32 size)
>  	int result;
>  	u8 cmd = CMD_DMA_EXT_WRITE;
>  
> -	/**
> -	 *      has to be greated than 4
> -	 **/
> +	/*
> +	 * has to be greated than 4
> +	 */
>  	if (size <= 4)
>  		return 0;
>  
> @@ -764,9 +770,9 @@ static int wilc_spi_write(struct wilc *wilc, u32 addr, u8 *buf, u32 size)
>  		return 0;
>  	}
>  
> -	/**
> -	 *      Data
> -	 **/
> +	/*
> +	 * Data
> +	 */
>  	result = spi_data_write(wilc, buf, size);
>  	if (result != N_OK)
>  		dev_err(&spi->dev, "Failed block data write...\n");
> @@ -783,7 +789,7 @@ static int wilc_spi_read_reg(struct wilc *wilc, u32 addr, u32 *data)
>  
>  	if (addr < 0x30) {
>  		/* dev_err(&spi->dev, "***** read addr %d\n\n", addr); */
> -		/* Clockless register*/
> +		/* Clockless register */
>  		cmd = CMD_INTERNAL_READ;
>  		clockless = 1;
>  	}
> @@ -825,9 +831,9 @@ static int wilc_spi_read(struct wilc *wilc, u32 addr, u8 *buf, u32 size)
>  
>  static int _wilc_spi_deinit(struct wilc *wilc)
>  {
> -	/**
> -	 *      TODO:
> -	 **/
> +	/*
> +	 * TODO:
> +	 */
>  	return 1;
>  }
>  
> @@ -849,15 +855,19 @@ static int wilc_spi_init(struct wilc *wilc, bool resume)
>  
>  	memset(&g_spi, 0, sizeof(struct wilc_spi));
>  
> -	/**
> -	 *      configure protocol
> -	 **/
> +	/*
> +	 * configure protocol
> +	 */
>  	g_spi.crc_off = 0;
>  
> -	/* TODO: We can remove the CRC trials if there is a definite way to reset */
> +	/*
> +	 * TODO: We can remove the CRC trials if there is a definite
> +	 * way to reset
> +	 */
>  	/* the SPI to it's initial value. */
>  	if (!spi_internal_read(wilc, WILC_SPI_PROTOCOL_OFFSET, &reg)) {
> -		/* Read failed. Try with CRC off. This might happen when module
> +		/*
> +		 * Read failed. Try with CRC off. This might happen when module
>  		 * is removed but chip isn't reset
>  		 */
>  		g_spi.crc_off = 1;
> @@ -870,7 +880,7 @@ static int wilc_spi_init(struct wilc *wilc, bool resume)
>  		}
>  	}
>  	if (g_spi.crc_off == 0)	{
> -		reg &= ~0xc;    /* disable crc checking */
> +		reg &= ~0xc; /* disable crc checking */
>  		reg &= ~0x70;
>  		reg |= (0x5 << 4);
>  		if (!spi_internal_write(wilc, WILC_SPI_PROTOCOL_OFFSET, reg)) {
> @@ -880,9 +890,9 @@ static int wilc_spi_init(struct wilc *wilc, bool resume)
>  		g_spi.crc_off = 1;
>  	}
>  
> -	/**
> -	 *      make sure can read back chip id correctly
> -	 **/
> +	/*
> +	 * make sure can read back chip id correctly
> +	 */
>  	if (!wilc_spi_read_reg(wilc, 0x1000, &chipid)) {
>  		dev_err(&spi->dev, "Fail cmd read chip id...\n");
>  		return 0;
> @@ -994,7 +1004,10 @@ static int wilc_spi_clear_int_ext(struct wilc *wilc, u32 val)
>  
>  			ret = 1;
>  			for (i = 0; i < g_spi.nint; i++) {
> -				/* No matter what you write 1 or 0, it will clear interrupt. */
> +				/*
> +				 * No matter what you write 1 or 0,
> +				 * it will clear interrupt.
> +				 */
>  				if (flags & 1)
>  					ret = wilc_spi_write_reg(wilc, 0x10c8 + i * 4, 1);
>  				if (!ret)
> @@ -1036,9 +1049,9 @@ static int wilc_spi_clear_int_ext(struct wilc *wilc, u32 val)
>  			}
>  
>  			if ((val & EN_VMM) == EN_VMM) {
> -				/**
> -				 *      enable vmm transfer.
> -				 **/
> +				/*
> +				 * enable vmm transfer.
> +				 */
>  				ret = wilc_spi_write_reg(wilc,
>  							 WILC_VMM_CORE_CTL, 1);
>  				if (!ret) {
> @@ -1065,9 +1078,9 @@ static int wilc_spi_sync_ext(struct wilc *wilc, int nint)
>  
>  	g_spi.nint = nint;
>  
> -	/**
> -	 *      interrupt pin mux select
> -	 **/
> +	/*
> +	 * interrupt pin mux select
> +	 */
>  	ret = wilc_spi_read_reg(wilc, WILC_PIN_MUX_0, &reg);
>  	if (!ret) {
>  		dev_err(&spi->dev, "Failed read reg (%08x)...\n",
> @@ -1082,9 +1095,9 @@ static int wilc_spi_sync_ext(struct wilc *wilc, int nint)
>  		return 0;
>  	}
>  
> -	/**
> -	 *      interrupt enable
> -	 **/
> +	/*
> +	 * interrupt enable
> +	 */
>  	ret = wilc_spi_read_reg(wilc, WILC_INTR_ENABLE, &reg);
>  	if (!ret) {
>  		dev_err(&spi->dev, "Failed read reg (%08x)...\n",
> -- 2.7.4

Re: [PATCH 3/6] staging: wilc1000: fix line over 80 characters in spi_cmd_complete()

[Claudiu Beznea]

On 14.02.2018 13:10, Ajay Singh wrote:
> Refactor spi_cmd_complete() to fix the line over 80 char issues reported
> by checkpatch.pl script.
> 
> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
> ---
>  drivers/staging/wilc1000/wilc_spi.c | 250 ++++++++++++++++++------------------
>  1 file changed, 124 insertions(+), 126 deletions(-)
> 
> diff --git a/drivers/staging/wilc1000/wilc_spi.c b/drivers/staging/wilc1000/wilc_spi.c
> index 66b6aea..30a9a61 100644
> --- a/drivers/staging/wilc1000/wilc_spi.c
> +++ b/drivers/staging/wilc1000/wilc_spi.c
> @@ -287,17 +287,19 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
>  	u8 rsp;
>  	int len = 0;
>  	int result = N_OK;
> +	int retry;
> +	u8 crc[2];
>  
>  	wb[0] = cmd;
>  	switch (cmd) {
> -	case CMD_SINGLE_READ:                           /* single word (4 bytes) read */
> +	case CMD_SINGLE_READ: /* single word (4 bytes) read */
Ok with this but I think would be nicer to have them documented the place were
they are defined.
>  		wb[1] = (u8)(adr >> 16);
>  		wb[2] = (u8)(adr >> 8);
>  		wb[3] = (u8)adr;
>  		len = 5;
>  		break;
>  
> -	case CMD_INTERNAL_READ:                 /* internal register read */
> +	case CMD_INTERNAL_READ: /* internal register read */
Ditto
>  		wb[1] = (u8)(adr >> 8);
>  		if (clockless == 1)
>  			wb[1] |= BIT(7);
> @@ -306,29 +308,29 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
>  		len = 5;
>  		break;
>  
> -	case CMD_TERMINATE:                                     /* termination */
> +	case CMD_TERMINATE:
Ditto
>  		wb[1] = 0x00;
>  		wb[2] = 0x00;
>  		wb[3] = 0x00;
>  		len = 5;
>  		break;
>  
> -	case CMD_REPEAT:                                                /* repeat */
> +	case CMD_REPEAT:
Ditto
>  		wb[1] = 0x00;
>  		wb[2] = 0x00;
>  		wb[3] = 0x00;
>  		len = 5;
>  		break;
>  
> -	case CMD_RESET:                                                 /* reset */
> +	case CMD_RESET:
Ditto
>  		wb[1] = 0xff;
>  		wb[2] = 0xff;
>  		wb[3] = 0xff;
>  		len = 5;
>  		break;
>  
> -	case CMD_DMA_WRITE:                                     /* dma write */
> -	case CMD_DMA_READ:                                      /* dma read */
> +	case CMD_DMA_WRITE: /* dma write */
> +	case CMD_DMA_READ:  /* dma read */
Ditto
>  		wb[1] = (u8)(adr >> 16);
>  		wb[2] = (u8)(adr >> 8);
>  		wb[3] = (u8)adr;
> @@ -337,8 +339,8 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
>  		len = 7;
>  		break;
>  
> -	case CMD_DMA_EXT_WRITE:         /* dma extended write */
> -	case CMD_DMA_EXT_READ:                  /* dma extended read */
> +	case CMD_DMA_EXT_WRITE: /* dma extended write */
> +	case CMD_DMA_EXT_READ:  /* dma extended read */
Ditto
>  		wb[1] = (u8)(adr >> 16);
>  		wb[2] = (u8)(adr >> 8);
>  		wb[3] = (u8)adr;
> @@ -348,7 +350,7 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
>  		len = 8;
>  		break;
>  
> -	case CMD_INTERNAL_WRITE:                /* internal register write */
> +	case CMD_INTERNAL_WRITE: /* internal register write */
Ditto
>  		wb[1] = (u8)(adr >> 8);
>  		if (clockless == 1)
>  			wb[1] |= BIT(7);
> @@ -360,7 +362,7 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
>  		len = 8;
>  		break;
>  
> -	case CMD_SINGLE_WRITE:                  /* single word write */
> +	case CMD_SINGLE_WRITE: /* single word write */
Ditto
>  		wb[1] = (u8)(adr >> 16);
>  		wb[2] = (u8)(adr >> 8);
>  		wb[3] = (u8)(adr);
> @@ -395,13 +397,12 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
>  	    cmd == CMD_REPEAT) {
>  		len2 = len + (NUM_SKIP_BYTES + NUM_RSP_BYTES + NUM_DUMMY_BYTES);
>  	} else if (cmd == CMD_INTERNAL_READ || cmd == CMD_SINGLE_READ) {
> -		if (!g_spi.crc_off) {
> -			len2 = len + (NUM_RSP_BYTES + NUM_DATA_HDR_BYTES + NUM_DATA_BYTES
> -				      + NUM_CRC_BYTES + NUM_DUMMY_BYTES);
> -		} else {
> -			len2 = len + (NUM_RSP_BYTES + NUM_DATA_HDR_BYTES + NUM_DATA_BYTES
> -				      + NUM_DUMMY_BYTES);
> -		}
> +		int tmp = NUM_RSP_BYTES + NUM_DATA_HDR_BYTES + NUM_DATA_BYTES
> +			+ NUM_DUMMY_BYTES;
> +		if (!g_spi.crc_off)
> +			len2 = len + tmp + NUM_CRC_BYTES;
> +		else
> +			len2 = len + tmp;
>  	} else {
>  		len2 = len + (NUM_RSP_BYTES + NUM_DUMMY_BYTES);
>  	}
> @@ -425,11 +426,8 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
>  	/*
>  	 * Command/Control response
>  	 */
> -	if (cmd == CMD_RESET ||
> -	    cmd == CMD_TERMINATE ||
> -	    cmd == CMD_REPEAT) {
> -		rix++;         /* skip 1 byte */
> -	}
> +	if (cmd == CMD_RESET || cmd == CMD_TERMINATE || cmd == CMD_REPEAT)
> +		rix++; /* skip 1 byte */
>  
>  	rsp = rb[rix++];
>  
> @@ -452,8 +450,6 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
>  
>  	if (cmd == CMD_INTERNAL_READ || cmd == CMD_SINGLE_READ ||
>  	    cmd == CMD_DMA_READ || cmd == CMD_DMA_EXT_READ) {
> -		int retry;
> -		u8 crc[2];
>  		/*
>  		 * Data Respnose header
>  		 */
> @@ -478,132 +474,134 @@ static int spi_cmd_complete(struct wilc *wilc, u8 cmd, u32 adr, u8 *b, u32 sz,
>  				"Error, data read response (%02x)\n", rsp);
>  			return N_RESET;
>  		}
> +	}
> +
> +	if (cmd == CMD_INTERNAL_READ || cmd == CMD_SINGLE_READ) {
> +		/*
> +		 * Read bytes
> +		 */
> +		if ((rix + 3) < len2) {
> +			b[0] = rb[rix++];
> +			b[1] = rb[rix++];
> +			b[2] = rb[rix++];
> +			b[3] = rb[rix++];
> +		} else {
> +			dev_err(&spi->dev,
> +				"buffer overrun when reading data.\n");
> +			return N_FAIL;
> +		}
>  
> -		if (cmd == CMD_INTERNAL_READ || cmd == CMD_SINGLE_READ) {
> +		if (!g_spi.crc_off) {
>  			/*
> -			 * Read bytes
> +			 * Read Crc
>  			 */
> -			if ((rix + 3) < len2) {
> -				b[0] = rb[rix++];
> -				b[1] = rb[rix++];
> -				b[2] = rb[rix++];
> -				b[3] = rb[rix++];
> +			if ((rix + 1) < len2) {
> +				crc[0] = rb[rix++];
> +				crc[1] = rb[rix++];
>  			} else {
>  				dev_err(&spi->dev,
> -					"buffer overrun when reading data.\n");
> +					"buffer overrun when reading crc.\n");
Since you refactor, you may try to have a consistent way of returning in case of
errors. I'm thinking that this kind of block:
{
	dev_err(&spi->dev, "msg");
	return N_FAIL;
}

could be replaced with:
- at the beginning of function:
const char *errmsg = NULL;

- and in case of error you could add this block:
{
	errmsg = "you error string";
	result = N_FAIL;
	goto _error_;
}

and for _error_ label:
_error_:
	if (errmsg)
		dev_err(&spi->dev, errmsg);
	return result;

And use this pattern wherever an error needs to be thrown, in a consistent way.
This could remain as is in this patch, for the moment, but further re-factors
might be needed for this function to look better, maybe should be added to backlog.

I want to say that might be nice to have a consistent way of returning with
errors, some paths in this function just uses "return error;" other paths are using:
result = error;
goto _error_;
and the result is the same.

>  				return N_FAIL;
>  			}
> +		}
> +	} else if ((cmd == CMD_DMA_READ) || (cmd == CMD_DMA_EXT_READ)) {
> +		int ix;
>  
> -			if (!g_spi.crc_off) {
> -				/*
> -				 * Read Crc
> -				 */
> -				if ((rix + 1) < len2) {
> -					crc[0] = rb[rix++];
> -					crc[1] = rb[rix++];
> -				} else {
> -					dev_err(&spi->dev, "buffer overrun when reading crc.\n");
> -					return N_FAIL;
> -				}
> -			}
> -		} else if ((cmd == CMD_DMA_READ) || (cmd == CMD_DMA_EXT_READ)) {
> -			int ix;
> -
> -			/* some data may be read in response to dummy bytes. */
> -			for (ix = 0; (rix < len2) && (ix < sz); )
> -				b[ix++] = rb[rix++];
> -
> -			sz -= ix;
> -
> -			if (sz > 0) {
> -				int nbytes;
> +		/* some data may be read in response to dummy bytes. */
> +		for (ix = 0; (rix < len2) && (ix < sz); )
> +			b[ix++] = rb[rix++];
>  
> -				if (sz <= (DATA_PKT_SZ - ix))
> -					nbytes = sz;
> -				else
> -					nbytes = DATA_PKT_SZ - ix;
> +		sz -= ix;
>  
> -				/*
> -				 * Read bytes
> -				 */
> -				if (wilc_spi_rx(wilc, &b[ix], nbytes)) {
> -					dev_err(&spi->dev, "Failed data block read, bus error...\n");
> -					result = N_FAIL;
> -					goto _error_;
> -				}
> +		if (sz > 0) {
> +			int nbytes;
>  
> -				/*
> -				 * Read Crc
> -				 */
> -				if (!g_spi.crc_off) {
> -					if (wilc_spi_rx(wilc, crc, 2)) {
> -						dev_err(&spi->dev, "Failed data block crc read, bus error...\n");
> -						result = N_FAIL;
> -						goto _error_;
> -					}
> -				}
> +			if (sz <= (DATA_PKT_SZ - ix))
> +				nbytes = sz;
> +			else
> +				nbytes = DATA_PKT_SZ - ix;
>  
> -				ix += nbytes;
> -				sz -= nbytes;
> +			/*
> +			 * Read bytes
> +			 */
> +			if (wilc_spi_rx(wilc, &b[ix], nbytes)) {
> +				dev_err(&spi->dev,
> +					"Failed block read, bus err\n");
> +				result = N_FAIL;
> +				goto _error_;
>  			}
>  
>  			/*
> -			 * if any data in left unread,
> -			 * then read the rest using normal DMA code.
> +			 * Read Crc
>  			 */
> -			while (sz > 0) {
> -				int nbytes;
> +			if (!g_spi.crc_off && wilc_spi_rx(wilc, crc, 2)) {
> +				dev_err(&spi->dev,
> +					"Failed block crc read, bus err\n");
> +				result = N_FAIL;
> +				goto _error_;
> +			}
>  
> -				if (sz <= DATA_PKT_SZ)
> -					nbytes = sz;
> -				else
> -					nbytes = DATA_PKT_SZ;
> +			ix += nbytes;
> +			sz -= nbytes;
> +		}
>  
> -				/*
> -				 * read data response only on the next DMA cycles not
> -				 * the first DMA since data response header is already
> -				 * handled above for the first DMA.
> -				 */
> -				/*
> -				 * Data Respnose header
> -				 */
> -				retry = 10;
> -				do {
> -					if (wilc_spi_rx(wilc, &rsp, 1)) {
> -						dev_err(&spi->dev, "Failed data response read, bus error...\n");
> -						result = N_FAIL;
> -						break;
> -					}
> -					if (((rsp >> 4) & 0xf) == 0xf)
> -						break;
> -				} while (retry--);
> -
> -				if (result == N_FAIL)
> -					break;
> +		/*
> +		 * if any data in left unread,
> +		 * then read the rest using normal DMA code.
> +		 */
> +		while (sz > 0) {
> +			int nbytes;
>  
> -				/*
> -				 * Read bytes
> -				 */
> -				if (wilc_spi_rx(wilc, &b[ix], nbytes)) {
> -					dev_err(&spi->dev, "Failed data block read, bus error...\n");
> +			if (sz <= DATA_PKT_SZ)
> +				nbytes = sz;
> +			else
> +				nbytes = DATA_PKT_SZ;
> +
> +			/*
> +			 * read data response only on the next DMA cycles not
> +			 * the first DMA since data response header is already
> +			 * handled above for the first DMA.
> +			 */
> +			/*
> +			 * Data Respnose header
> +			 */
> +			retry = 10;
> +			do {
> +				if (wilc_spi_rx(wilc, &rsp, 1)) {
> +					dev_err(&spi->dev,
> +						"Failed resp read, bus err\n");
>  					result = N_FAIL;
>  					break;
Here, also, the proposed solution could be used. Or just return as in the other
cases or just goto _error_ instead of break and then check for result == N_FAIL.
>  				}
> +				if (((rsp >> 4) & 0xf) == 0xf)
> +					break;
> +			} while (retry--);
>  
> -				/*
> -				 * Read Crc
> -				 */
> -				if (!g_spi.crc_off) {
> -					if (wilc_spi_rx(wilc, crc, 2)) {
> -						dev_err(&spi->dev, "Failed data block crc read, bus error...\n");
> -						result = N_FAIL;
> -						break;
> -					}
> -				}
> +			if (result == N_FAIL)
> +				break;
>  
> -				ix += nbytes;
> -				sz -= nbytes;
> +			/*
> +			 * Read bytes
> +			 */
> +			if (wilc_spi_rx(wilc, &b[ix], nbytes)) {
> +				dev_err(&spi->dev,
> +					"Failed block read, bus err\n");
> +				result = N_FAIL;
> +				break;
>  			}
> +
> +			/*
> +			 * Read Crc
> +			 */
> +			if (!g_spi.crc_off && wilc_spi_rx(wilc, crc, 2)) {
> +				dev_err(&spi->dev,
> +					"Failed block crc read, bus err\n");
> +				result = N_FAIL;
> +				break;
> +			}
> +
> +			ix += nbytes;
> +			sz -= nbytes;
>  		}
>  	}
>  _error_:
> 

Re: [PATCH 5/6] staging: wilc1000: fix line over 80 characters in wilc_spi_read_int()

[Claudiu Beznea]

On 14.02.2018 13:10, Ajay Singh wrote:
> Refactor wilc_spi_read_int() to fix the line over 80 char issues reported
> by checkpatch.pl script.
> 
> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
> ---
>  drivers/staging/wilc1000/wilc_spi.c | 57 +++++++++++++++++++------------------
>  1 file changed, 29 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/staging/wilc1000/wilc_spi.c b/drivers/staging/wilc1000/wilc_spi.c
> index fddc0db..7c58beb8 100644
> --- a/drivers/staging/wilc1000/wilc_spi.c
> +++ b/drivers/staging/wilc1000/wilc_spi.c
> @@ -939,45 +939,46 @@ static int wilc_spi_read_int(struct wilc *wilc, u32 *int_status)
>  	int happened, j;
>  	u32 unknown_mask;
>  	u32 irq_flags;
> +	int k = IRG_FLAGS_OFFSET + 5;
>  
>  	if (g_spi.has_thrpt_enh) {
>  		ret = spi_internal_read(wilc, 0xe840 - WILC_SPI_REG_BASE,
>  					int_status);
> -	} else {
> -		ret = wilc_spi_read_reg(wilc, WILC_VMM_TO_HOST_SIZE,
> -					&byte_cnt);
> -		if (!ret) {
> -			dev_err(&spi->dev,
> -				"Failed read WILC_VMM_TO_HOST_SIZE ...\n");
> -			goto _fail_;
> -		}
> -		tmp = (byte_cnt >> 2) & IRQ_DMA_WD_CNT_MASK;
> +		return ret;
> +	}
> +	ret = wilc_spi_read_reg(wilc, WILC_VMM_TO_HOST_SIZE, &byte_cnt);
> +	if (!ret) {
> +		dev_err(&spi->dev,
> +			"Failed read WILC_VMM_TO_HOST_SIZE ...\n");
> +		goto _fail_;
> +	}
> +	tmp = (byte_cnt >> 2) & IRQ_DMA_WD_CNT_MASK;
>  
> -		j = 0;
> -		do {
> -			happened = 0;
> +	j = 0;
> +	do {
> +		happened = 0;
You could remove this happen
>  
> -			wilc_spi_read_reg(wilc, 0x1a90, &irq_flags);
> -			tmp |= ((irq_flags >> 27) << IRG_FLAGS_OFFSET);
> +		wilc_spi_read_reg(wilc, 0x1a90, &irq_flags);
> +		tmp |= ((irq_flags >> 27) << IRG_FLAGS_OFFSET);
>  
> -			if (g_spi.nint > 5) {
> -				wilc_spi_read_reg(wilc, 0x1a94,
> -						  &irq_flags);
> -				tmp |= (((irq_flags >> 0) & 0x7) << (IRG_FLAGS_OFFSET + 5));
> -			}
> +		if (g_spi.nint > 5) {
> +			wilc_spi_read_reg(wilc, 0x1a94, &irq_flags);
> +			tmp |= (((irq_flags >> 0) & 0x7) << k);
> +		}
>  
> -			unknown_mask = ~((1ul << g_spi.nint) - 1);
> +		unknown_mask = ~((1ul << g_spi.nint) - 1);
You could use GENMASK(g_spi.nint - 1, 0) instead of ~((1ul << g_spi.nint) - 1)
>  
> -			if ((tmp >> IRG_FLAGS_OFFSET) & unknown_mask) {
> -				dev_err(&spi->dev, "Unexpected interrupt (2): j=%d, tmp=%x, mask=%x\n", j, tmp, unknown_mask);
> -					happened = 1;
> -			}
> +		if ((tmp >> IRG_FLAGS_OFFSET) & unknown_mask) {
> +			dev_err(&spi->dev,
> +				"Unexpected interrupt(2):j=%d,tmp=%x,mask=%x\n",
> +				j, tmp, unknown_mask);
> +				happened = 1;
And here just break;
> +		}
>  
> -			j++;
> -		} while (happened);
And here use while (true);
> +		j++;
> +	} while (happened);
>  
> -		*int_status = tmp;
> -	}
> +	*int_status = tmp;
>  
>  _fail_:
>  	return ret;
> 

Re: [PATCH 6/6] staging: wilc1000: fix line over 80 chars in wilc_spi_clear_int_ext()

[Claudiu Beznea]

On 14.02.2018 13:10, Ajay Singh wrote:
> Refactor wilc_spi_clear_int_ext() to fix the "line over 80 char" issue
> reported by checkpatch.pl script.
> 
> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
> ---
>  drivers/staging/wilc1000/wilc_spi.c | 113 +++++++++++++++++-------------------
>  1 file changed, 54 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/staging/wilc1000/wilc_spi.c b/drivers/staging/wilc1000/wilc_spi.c
> index 7c58beb8..6b392c9 100644
> --- a/drivers/staging/wilc1000/wilc_spi.c
> +++ b/drivers/staging/wilc1000/wilc_spi.c
> @@ -988,74 +988,69 @@ static int wilc_spi_clear_int_ext(struct wilc *wilc, u32 val)
>  {
>  	struct spi_device *spi = to_spi_device(wilc->dev);
>  	int ret;
> +	u32 flags;
> +	u32 tbl_ctl;
>  
>  	if (g_spi.has_thrpt_enh) {
>  		ret = spi_internal_write(wilc, 0xe844 - WILC_SPI_REG_BASE,
>  					 val);
> -	} else {
> -		u32 flags;
> -
> -		flags = val & (BIT(MAX_NUM_INT) - 1);> -		if (flags) {
> -			int i;
> -
> -			ret = 1;
> -			for (i = 0; i < g_spi.nint; i++) {
> -				/*
> -				 * No matter what you write 1 or 0,
> -				 * it will clear interrupt.
> -				 */
> -				if (flags & 1)
> -					ret = wilc_spi_write_reg(wilc, 0x10c8 + i * 4, 1);
> -				if (!ret)
> -					break;
> -				flags >>= 1;
> -			}
> -			if (!ret) {
> +		return ret;
> +	}
> +
> +	flags = val & (BIT(MAX_NUM_INT) - 1);

Or you could use:
	unsigned long expected_irqs, unexpected_irqs;

	expected_irqs = val & GENMASK(g_spi.int - 1, 0);
	unexpected_irq = val & GENMASK(MAX_NUM_INT - 1, g_spi.int);

	for (i = 0; i < g_spi.nint && expected_irqs; i++) {
		if (expected_irqs & BIT(i)) {
			ret = wilc_spi_write_reg(wilc, 0x10c8 + i * 4, 1);
			if (ret) {
				dev_err(...);
				goto _fail_;
			}
		}
	}

	for (i = g_spi.nint; i < MAX_NUM_INT && unexpected_irq; i++) {
		if (unexpected_irqs & BIT(i))
			dev_err(...);

Instead of this:
> +	if (flags) {
> +		int i;
> +
> +		ret = 1;
> +		for (i = 0; i < g_spi.nint; i++) {
> +			/*
> +			 * No matter what you write 1 or 0,
> +			 * it will clear interrupt.
> +			 */> +			if (flags & 1)
> +				ret = wilc_spi_write_reg(wilc,
> +							 0x10c8 + i * 4, 1);
> +			if (!ret)
> +				break;
> +			flags >>= 1;
> +		}
> +		if (!ret) {
> +			dev_err(&spi->dev,
> +				"Failed wilc_spi_write_reg, set reg %x ...\n",
> +				0x10c8 + i * 4);
> +			goto _fail_;
> +		}
> +		for (i = g_spi.nint; i < MAX_NUM_INT; i++) {
> +			if (flags & 1)
>  				dev_err(&spi->dev,
> -					"Failed wilc_spi_write_reg, set reg %x ...\n",
> -					0x10c8 + i * 4);
> -				goto _fail_;
> -			}
> -			for (i = g_spi.nint; i < MAX_NUM_INT; i++) {
> -				if (flags & 1)
> -					dev_err(&spi->dev,
> -						"Unexpected interrupt cleared %d...\n",
> -						i);
> -				flags >>= 1;
> -			}
> +					"Unexpected interrupt cleared %d...\n",
> +					i);
> +			flags >>= 1;
>  		}
> +	}
>  
until here.

> -		{
> -			u32 tbl_ctl;
> -
> -			tbl_ctl = 0;
> -			/* select VMM table 0 */
> -			if ((val & SEL_VMM_TBL0) == SEL_VMM_TBL0)
> -				tbl_ctl |= BIT(0);
> -			/* select VMM table 1 */
> -			if ((val & SEL_VMM_TBL1) == SEL_VMM_TBL1)
> -				tbl_ctl |= BIT(1);
> +	tbl_ctl = 0;
> +	/* select VMM table 0 */
> +	if ((val & SEL_VMM_TBL0) == SEL_VMM_TBL0)
> +		tbl_ctl |= BIT(0);
> +	/* select VMM table 1 */
> +	if ((val & SEL_VMM_TBL1) == SEL_VMM_TBL1)
> +		tbl_ctl |= BIT(1);
>  
> -			ret = wilc_spi_write_reg(wilc, WILC_VMM_TBL_CTL,
> -						 tbl_ctl);
> -			if (!ret) {
> -				dev_err(&spi->dev,
> -					"fail write reg vmm_tbl_ctl...\n");
> -				goto _fail_;
> -			}
> +	ret = wilc_spi_write_reg(wilc, WILC_VMM_TBL_CTL, tbl_ctl);
> +	if (!ret) {
> +		dev_err(&spi->dev, "fail write reg vmm_tbl_ctl...\n");
> +		goto _fail_;
> +	}
>  
> -			if ((val & EN_VMM) == EN_VMM) {
> -				/*
> -				 * enable vmm transfer.
> -				 */
> -				ret = wilc_spi_write_reg(wilc,
> -							 WILC_VMM_CORE_CTL, 1);
> -				if (!ret) {
> -					dev_err(&spi->dev, "fail write reg vmm_core_ctl...\n");
> -					goto _fail_;
> -				}
> -			}
> +	if ((val & EN_VMM) == EN_VMM) {
> +		/*
> +		 * enable vmm transfer.
> +		 */> +		ret = wilc_spi_write_reg(wilc, WILC_VMM_CORE_CTL, 1);
> +		if (!ret) {
> +			dev_err(&spi->dev, "fail write reg vmm_core_ctl...\n");
> +			goto _fail_;
>  		}
>  	}
>  _fail_:
> 

Re: [PATCH 4/6] staging: wilc1000: fix line over 80 characters in wilc_spi_init()

[Ajay Singh]

On Fri, 16 Feb 2018 15:30:22 +0100
Greg KH <gregkh@linuxfoundation.org> wrote:

> > diff --git a/drivers/staging/wilc1000/wilc_spi.c b/drivers/staging/wilc1000/wilc_spi.c
> > index 30a9a61..fddc0db 100644
> > --- a/drivers/staging/wilc1000/wilc_spi.c
> > +++ b/drivers/staging/wilc1000/wilc_spi.c
> > @@ -835,7 +835,6 @@ static int wilc_spi_init(struct wilc *wilc, bool resume)
> >  	struct spi_device *spi = to_spi_device(wilc->dev);
> >  	u32 reg;
> >  	u32 chipid;
> > -
> >  	static int isinit;  
> 
> You also deleted this line :(

The blank line was not necessary between the variable's declaration. I
should have done these changes in separate patch.
I will take care to have these type of changes either in a separate patch
or include the change details in description.

Re: [PATCH 6/6] staging: wilc1000: fix line over 80 chars in wilc_spi_clear_int_ext()

[Ajay Singh]

On Fri, 16 Feb 2018 20:16:02 +0200
Claudiu Beznea <Claudiu.Beznea@microchip.com> wrote:

> Or you could use:
> 	unsigned long expected_irqs, unexpected_irqs;
> 
> 	expected_irqs = val & GENMASK(g_spi.int - 1, 0);
> 	unexpected_irq = val & GENMASK(MAX_NUM_INT - 1, g_spi.int);
> 
> 	for (i = 0; i < g_spi.nint && expected_irqs; i++) {
> 		if (expected_irqs & BIT(i)) {
> 			ret = wilc_spi_write_reg(wilc, 0x10c8 + i * 4, 1);
> 			if (ret) {
> 				dev_err(...);
> 				goto _fail_;
> 			}
> 		}
> 	}
> 
> 	for (i = g_spi.nint; i < MAX_NUM_INT && unexpected_irq; i++) {
> 		if (unexpected_irqs & BIT(i))
> 			dev_err(...);
> 

Thanks for suggestion.
I will take this input and make use of GENMASK macro to modify the
function. In a separate patch will submit these changes. As there are
other functions,where same macro can be used so will include them
together in separate patch.

Re: [PATCH 5/6] staging: wilc1000: fix line over 80 characters in wilc_spi_read_int()

[Ajay Singh]

On Fri, 16 Feb 2018 20:15:53 +0200
Claudiu Beznea <Claudiu.Beznea@microchip.com> wrote:

> On 14.02.2018 13:10, Ajay Singh wrote:
> > Refactor wilc_spi_read_int() to fix the line over 80 char issues reported
> > by checkpatch.pl script.
> > 
> > Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
> > ---
> >  drivers/staging/wilc1000/wilc_spi.c | 57 +++++++++++++++++++------------------
> >  1 file changed, 29 insertions(+), 28 deletions(-)
> > 
> > diff --git a/drivers/staging/wilc1000/wilc_spi.c b/drivers/staging/wilc1000/wilc_spi.c
> > index fddc0db..7c58beb8 100644
> > --- a/drivers/staging/wilc1000/wilc_spi.c
> > +++ b/drivers/staging/wilc1000/wilc_spi.c
> > @@ -939,45 +939,46 @@ static int wilc_spi_read_int(struct wilc *wilc, u32 *int_status)
> >  	int happened, j;
> >  	u32 unknown_mask;
> >  	u32 irq_flags;
> > +	int k = IRG_FLAGS_OFFSET + 5;
> >  
> >  	if (g_spi.has_thrpt_enh) {
> >  		ret = spi_internal_read(wilc, 0xe840 - WILC_SPI_REG_BASE,
> >  					int_status);
> > -	} else {
> > -		ret = wilc_spi_read_reg(wilc, WILC_VMM_TO_HOST_SIZE,
> > -					&byte_cnt);
> > -		if (!ret) {
> > -			dev_err(&spi->dev,
> > -				"Failed read WILC_VMM_TO_HOST_SIZE ...\n");
> > -			goto _fail_;
> > -		}
> > -		tmp = (byte_cnt >> 2) & IRQ_DMA_WD_CNT_MASK;
> > +		return ret;
> > +	}
> > +	ret = wilc_spi_read_reg(wilc, WILC_VMM_TO_HOST_SIZE, &byte_cnt);
> > +	if (!ret) {
> > +		dev_err(&spi->dev,
> > +			"Failed read WILC_VMM_TO_HOST_SIZE ...\n");
> > +		goto _fail_;
> > +	}
> > +	tmp = (byte_cnt >> 2) & IRQ_DMA_WD_CNT_MASK;
> >  
> > -		j = 0;
> > -		do {
> > -			happened = 0;
> > +	j = 0;
> > +	do {
> > +		happened = 0;  
> You could remove this happen
> >  

Yes, we don't need "happened" variable,it can be removed. As the patch
was only to remove the 80 char checkpatch.pl warning. Will remove the
use of this variable in separate patch.

> > -			wilc_spi_read_reg(wilc, 0x1a90, &irq_flags);
> > -			tmp |= ((irq_flags >> 27) << IRG_FLAGS_OFFSET);
> > +		wilc_spi_read_reg(wilc, 0x1a90, &irq_flags);
> > +		tmp |= ((irq_flags >> 27) << IRG_FLAGS_OFFSET);
> >  
> > -			if (g_spi.nint > 5) {
> > -				wilc_spi_read_reg(wilc, 0x1a94,
> > -						  &irq_flags);
> > -				tmp |= (((irq_flags >> 0) & 0x7) << (IRG_FLAGS_OFFSET + 5));
> > -			}
> > +		if (g_spi.nint > 5) {
> > +			wilc_spi_read_reg(wilc, 0x1a94, &irq_flags);
> > +			tmp |= (((irq_flags >> 0) & 0x7) << k);
> > +		}
> >  
> > -			unknown_mask = ~((1ul << g_spi.nint) - 1);
> > +		unknown_mask = ~((1ul << g_spi.nint) - 1);  
> You could use GENMASK(g_spi.nint - 1, 0) instead of ~((1ul << g_spi.nint) - 1)
> >  

Will do the modification to make use of GENMASK in future patch.

> > -			if ((tmp >> IRG_FLAGS_OFFSET) & unknown_mask) {
> > -				dev_err(&spi->dev, "Unexpected interrupt (2): j=%d, tmp=%x, mask=%x\n", j, tmp, unknown_mask);
> > -					happened = 1;
> > -			}
> > +		if ((tmp >> IRG_FLAGS_OFFSET) & unknown_mask) {
> > +			dev_err(&spi->dev,
> > +				"Unexpected interrupt(2):j=%d,tmp=%x,mask=%x\n",
> > +				j, tmp, unknown_mask);
> > +				happened = 1;  
> And here just break;
> > +		}
> >  
> > -			j++;
> > -		} while (happened);  
> And here use while (true);
> > +		j++;
> > +	} while (happened);
> >  
> > -		*int_status = tmp;
> > -	}
> > +	*int_status = tmp;
> >  
> >  _fail_:
> >  	return ret;
> >   



regards,
ajay