* [PATCH 1/6] staging: wilc1000: modified code comments as per linux coding style
2018-02-14 11:10 [PATCH 0/6] fix line over 80 char & coding style in wilc_spi.c Ajay Singh
@ 2018-02-14 11:10 ` Ajay Singh
2018-02-16 18:15 ` Claudiu Beznea
2018-02-14 11:10 ` [PATCH 2/6] staging: wilc1000: removed the unnecessary commented code Ajay Singh
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Ajay Singh @ 2018-02-14 11:10 UTC (permalink / raw)
To: linux-wireless
Cc: devel, gregkh, ganesh.krishna, venkateswara.kaja, aditya.shankar,
claudiu.beznea, 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, ®)) {
- /* 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, ®);
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, ®);
if (!ret) {
dev_err(&spi->dev, "Failed read reg (%08x)...\n",
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 1/6] staging: wilc1000: modified code comments as per linux coding style
2018-02-14 11:10 ` [PATCH 1/6] staging: wilc1000: modified code comments as per linux coding style Ajay Singh
@ 2018-02-16 18:15 ` Claudiu Beznea
0 siblings, 0 replies; 15+ messages in thread
From: Claudiu Beznea @ 2018-02-16 18:15 UTC (permalink / raw)
To: Ajay Singh, linux-wireless
Cc: devel, gregkh, ganesh.krishna, venkateswara.kaja, aditya.shankar
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, ®)) {
> - /* 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, ®);
> 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, ®);
> if (!ret) {
> dev_err(&spi->dev, "Failed read reg (%08x)...\n",
> -- 2.7.4
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/6] staging: wilc1000: removed the unnecessary commented code
2018-02-14 11:10 [PATCH 0/6] fix line over 80 char & coding style in wilc_spi.c Ajay Singh
2018-02-14 11:10 ` [PATCH 1/6] staging: wilc1000: modified code comments as per linux coding style Ajay Singh
@ 2018-02-14 11:10 ` Ajay Singh
2018-02-14 11:10 ` [PATCH 3/6] staging: wilc1000: fix line over 80 characters in spi_cmd_complete() Ajay Singh
` (3 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Ajay Singh @ 2018-02-14 11:10 UTC (permalink / raw)
To: linux-wireless
Cc: devel, gregkh, ganesh.krishna, venkateswara.kaja, aditya.shankar,
claudiu.beznea, 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 3/6] staging: wilc1000: fix line over 80 characters in spi_cmd_complete()
2018-02-14 11:10 [PATCH 0/6] fix line over 80 char & coding style in wilc_spi.c Ajay Singh
2018-02-14 11:10 ` [PATCH 1/6] staging: wilc1000: modified code comments as per linux coding style Ajay Singh
2018-02-14 11:10 ` [PATCH 2/6] staging: wilc1000: removed the unnecessary commented code Ajay Singh
@ 2018-02-14 11:10 ` Ajay Singh
2018-02-16 18:15 ` Claudiu Beznea
2018-02-14 11:10 ` [PATCH 4/6] staging: wilc1000: fix line over 80 characters in wilc_spi_init() Ajay Singh
` (2 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Ajay Singh @ 2018-02-14 11:10 UTC (permalink / raw)
To: linux-wireless
Cc: devel, gregkh, ganesh.krishna, venkateswara.kaja, aditya.shankar,
claudiu.beznea, 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 3/6] staging: wilc1000: fix line over 80 characters in spi_cmd_complete()
2018-02-14 11:10 ` [PATCH 3/6] staging: wilc1000: fix line over 80 characters in spi_cmd_complete() Ajay Singh
@ 2018-02-16 18:15 ` Claudiu Beznea
0 siblings, 0 replies; 15+ messages in thread
From: Claudiu Beznea @ 2018-02-16 18:15 UTC (permalink / raw)
To: Ajay Singh, linux-wireless
Cc: devel, gregkh, ganesh.krishna, venkateswara.kaja, aditya.shankar
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_:
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/6] staging: wilc1000: fix line over 80 characters in wilc_spi_init()
2018-02-14 11:10 [PATCH 0/6] fix line over 80 char & coding style in wilc_spi.c Ajay Singh
` (2 preceding siblings ...)
2018-02-14 11:10 ` [PATCH 3/6] staging: wilc1000: fix line over 80 characters in spi_cmd_complete() Ajay Singh
@ 2018-02-14 11:10 ` Ajay Singh
2018-02-16 14:30 ` Greg KH
2018-02-14 11:10 ` [PATCH 5/6] staging: wilc1000: fix line over 80 characters in wilc_spi_read_int() Ajay Singh
2018-02-14 11:10 ` [PATCH 6/6] staging: wilc1000: fix line over 80 chars in wilc_spi_clear_int_ext() Ajay Singh
5 siblings, 1 reply; 15+ messages in thread
From: Ajay Singh @ 2018-02-14 11:10 UTC (permalink / raw)
To: linux-wireless
Cc: devel, gregkh, ganesh.krishna, venkateswara.kaja, aditya.shankar,
claudiu.beznea, 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, ®)) {
- /* 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 4/6] staging: wilc1000: fix line over 80 characters in wilc_spi_init()
2018-02-14 11:10 ` [PATCH 4/6] staging: wilc1000: fix line over 80 characters in wilc_spi_init() Ajay Singh
@ 2018-02-16 14:30 ` Greg KH
2018-02-19 6:37 ` Ajay Singh
0 siblings, 1 reply; 15+ messages in thread
From: Greg KH @ 2018-02-16 14:30 UTC (permalink / raw)
To: Ajay Singh
Cc: linux-wireless, devel, venkateswara.kaja, ganesh.krishna,
aditya.shankar
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 :(
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/6] staging: wilc1000: fix line over 80 characters in wilc_spi_init()
2018-02-16 14:30 ` Greg KH
@ 2018-02-19 6:37 ` Ajay Singh
0 siblings, 0 replies; 15+ messages in thread
From: Ajay Singh @ 2018-02-19 6:37 UTC (permalink / raw)
To: Greg KH
Cc: linux-wireless, devel, venkateswara.kaja, ganesh.krishna,
aditya.shankar
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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 5/6] staging: wilc1000: fix line over 80 characters in wilc_spi_read_int()
2018-02-14 11:10 [PATCH 0/6] fix line over 80 char & coding style in wilc_spi.c Ajay Singh
` (3 preceding siblings ...)
2018-02-14 11:10 ` [PATCH 4/6] staging: wilc1000: fix line over 80 characters in wilc_spi_init() Ajay Singh
@ 2018-02-14 11:10 ` Ajay Singh
2018-02-16 18:15 ` Claudiu Beznea
2018-02-14 11:10 ` [PATCH 6/6] staging: wilc1000: fix line over 80 chars in wilc_spi_clear_int_ext() Ajay Singh
5 siblings, 1 reply; 15+ messages in thread
From: Ajay Singh @ 2018-02-14 11:10 UTC (permalink / raw)
To: linux-wireless
Cc: devel, gregkh, ganesh.krishna, venkateswara.kaja, aditya.shankar,
claudiu.beznea, 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 5/6] staging: wilc1000: fix line over 80 characters in wilc_spi_read_int()
2018-02-14 11:10 ` [PATCH 5/6] staging: wilc1000: fix line over 80 characters in wilc_spi_read_int() Ajay Singh
@ 2018-02-16 18:15 ` Claudiu Beznea
2018-02-19 8:04 ` Ajay Singh
0 siblings, 1 reply; 15+ messages in thread
From: Claudiu Beznea @ 2018-02-16 18:15 UTC (permalink / raw)
To: Ajay Singh, linux-wireless
Cc: devel, gregkh, ganesh.krishna, venkateswara.kaja, aditya.shankar
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;
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 5/6] staging: wilc1000: fix line over 80 characters in wilc_spi_read_int()
2018-02-16 18:15 ` Claudiu Beznea
@ 2018-02-19 8:04 ` Ajay Singh
0 siblings, 0 replies; 15+ messages in thread
From: Ajay Singh @ 2018-02-19 8:04 UTC (permalink / raw)
To: Claudiu Beznea
Cc: linux-wireless, devel, gregkh, ganesh.krishna, venkateswara.kaja,
aditya.shankar, Adham.Abozaeid
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 6/6] staging: wilc1000: fix line over 80 chars in wilc_spi_clear_int_ext()
2018-02-14 11:10 [PATCH 0/6] fix line over 80 char & coding style in wilc_spi.c Ajay Singh
` (4 preceding siblings ...)
2018-02-14 11:10 ` [PATCH 5/6] staging: wilc1000: fix line over 80 characters in wilc_spi_read_int() Ajay Singh
@ 2018-02-14 11:10 ` Ajay Singh
2018-02-16 18:16 ` Claudiu Beznea
5 siblings, 1 reply; 15+ messages in thread
From: Ajay Singh @ 2018-02-14 11:10 UTC (permalink / raw)
To: linux-wireless
Cc: devel, gregkh, ganesh.krishna, venkateswara.kaja, aditya.shankar,
claudiu.beznea, 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 6/6] staging: wilc1000: fix line over 80 chars in wilc_spi_clear_int_ext()
2018-02-14 11:10 ` [PATCH 6/6] staging: wilc1000: fix line over 80 chars in wilc_spi_clear_int_ext() Ajay Singh
@ 2018-02-16 18:16 ` Claudiu Beznea
2018-02-19 7:49 ` Ajay Singh
0 siblings, 1 reply; 15+ messages in thread
From: Claudiu Beznea @ 2018-02-16 18:16 UTC (permalink / raw)
To: Ajay Singh, linux-wireless
Cc: devel, gregkh, ganesh.krishna, venkateswara.kaja, aditya.shankar
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_:
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 6/6] staging: wilc1000: fix line over 80 chars in wilc_spi_clear_int_ext()
2018-02-16 18:16 ` Claudiu Beznea
@ 2018-02-19 7:49 ` Ajay Singh
0 siblings, 0 replies; 15+ messages in thread
From: Ajay Singh @ 2018-02-19 7:49 UTC (permalink / raw)
To: Claudiu Beznea
Cc: linux-wireless, devel, gregkh, ganesh.krishna, venkateswara.kaja,
aditya.shankar, Adham.Abozaeid
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.
^ permalink raw reply [flat|nested] 15+ messages in thread