[PATCH 3/7] staging/wilc1000: Sanitize linux_spi_read
Removed unneeded newlines. Moved variable definitions to the top. Improved control flow to get rid of indents. Replaced while with for. Added additional error message and improved existing ones. Signed-off-by: Janosch Frank --- drivers/staging/wilc1000/linux_wlan_spi.c | 79 +++ 1 file changed, 39 insertions(+), 40 deletions(-) diff --git a/drivers/staging/wilc1000/linux_wlan_spi.c b/drivers/staging/wilc1000/linux_wlan_spi.c index 595e274..b0dd486 100644 --- a/drivers/staging/wilc1000/linux_wlan_spi.c +++ b/drivers/staging/wilc1000/linux_wlan_spi.c @@ -218,70 +218,69 @@ int linux_spi_write(u8 *b, u32 len) int linux_spi_read(u8 *rb, u32 rlen) { - int ret; + int ret, i; + int blk = rlen / TXRX_PHASE_SIZE; + int remainder = rlen % TXRX_PHASE_SIZE; struct spi_message msg; struct spi_transfer tr; - if (rlen > 0) { - int i = 0; - - int blk = rlen / TXRX_PHASE_SIZE; - int remainder = rlen % TXRX_PHASE_SIZE; + if (!rlen) { + PRINT_ER("Zero length read.\n"); + return 0; + } - if (blk) { - while (i < blk) { - linux_spi_msg_init(&msg, &tr, TXRX_PHASE_SIZE, - NULL, - rb + (i * TXRX_PHASE_SIZE)); + if (!rb) { + PRINT_ER("Read buffer NULL.\n"); + return 0; + } - ret = spi_sync(wilc_spi_dev, &msg); - if (ret < 0) { - PRINT_ER("SPI transaction failed\n"); - } - i++; - } + for (i = 0; i < blk; i++) { + linux_spi_msg_init(&msg, &tr, TXRX_PHASE_SIZE, NULL, + rb + (i * TXRX_PHASE_SIZE)); + ret = spi_sync(wilc_spi_dev, &msg); + if (ret < 0) { + PRINT_ER("SPI sync failed and returned %d.\n", ret); + return 0; } - if (remainder) { - linux_spi_msg_init(&msg, &tr, remainder, NULL, - rb + (blk * TXRX_PHASE_SIZE)); + } - ret = spi_sync(wilc_spi_dev, &msg); - if (ret < 0) { - PRINT_ER("SPI transaction failed\n"); - } - } - } else { - PRINT_ER("can't read data with the following length: %u\n", rlen); - ret = -1; + if (remainder) { + linux_spi_msg_init(&msg, &tr, remainder, NULL, + rb + (blk * TXRX_PHASE_SIZE)); + ret = spi_sync(wilc_spi_dev, &msg); + if (ret < 0) + PRINT_ER("SPI sync failed and returned %d.\n", ret); } + /* change return value to match WILC interface */ (ret < 0) ? (ret = 0) : (ret = 1); - return ret; } #else int linux_spi_read(u8 *rb, u32 rlen) { - int ret; struct spi_message msg; struct spi_transfer tr; - if (rlen > 0) { - linux_spi_msg_init(&msg, &tr, rlen, NULL, rb); + if (!rlen) { + PRINT_ER("Zero length read.\n"); + return 0; + } - ret = spi_sync(wilc_spi_dev, &msg); - if (ret < 0) { - PRINT_ER("SPI transaction failed\n"); - } - } else { - PRINT_ER("can't read data with the following length: %u\n", rlen); - ret = -1; + if (!rb) { + PRINT_ER("Read buffer NULL.\n"); + return 0; } + + linux_spi_msg_init(&msg, &tr, rlen, NULL, rb); + ret = spi_sync(wilc_spi_dev, &msg); + if (ret < 0) + PRINT_ER("SPI sync failed and returned %d.\n", ret); + /* change return value to match WILC interface */ (ret < 0) ? (ret = 0) : (ret = 1); - return ret; } -- 2.5.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/7] staging/wilc1000: Sanitize linux_spi_write_read
Removed unneeded newlines. Improved control flow to get rid of indents. Added additional error message and improved existing ones. Signed-off-by: Janosch Frank --- drivers/staging/wilc1000/linux_wlan_spi.c | 24 +--- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/drivers/staging/wilc1000/linux_wlan_spi.c b/drivers/staging/wilc1000/linux_wlan_spi.c index 72230d2..595e274 100644 --- a/drivers/staging/wilc1000/linux_wlan_spi.c +++ b/drivers/staging/wilc1000/linux_wlan_spi.c @@ -289,25 +289,27 @@ int linux_spi_read(u8 *rb, u32 rlen) int linux_spi_write_read(u8 *wb, u8 *rb, u32 rlen) { - int ret; struct spi_message msg; struct spi_transfer tr; - if (rlen > 0) { - linux_spi_msg_init(&msg, &tr, rlen, wb, rb); + if (!rlen) { + PRINT_ER("Zero length read/write.\n"); + return 0; + } - ret = spi_sync(wilc_spi_dev, &msg); - if (ret < 0) { - PRINT_ER("SPI transaction failed\n"); - } - } else { - PRINT_ER("can't read data with the following length: %u\n", rlen); - ret = -1; + if (!wb || !rb) { + PRINT_ER("Read or write buffer NULL.\n"); + return 0; } + + linux_spi_msg_init(&msg, &tr, rlen, wb, rb); + ret = spi_sync(wilc_spi_dev, &msg); + if (ret < 0) + PRINT_ER("SPI sync failed and returned %d.\n", ret); + /* change return value to match WILC interface */ (ret < 0) ? (ret = 0) : (ret = 1); - return ret; } -- 2.5.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/7] staging/wilc1000: Refactor spi read and write functions
The code for sending and receiving spi data had multiple style problems, was repetative and contained errors. This series simplifies the code and fixes some of the problems. Janosch Frank (7): staging/wilc1000: Introduce linux_spi_msg_init staging/wilc1000: Sanitize linux_spi_write_read staging/wilc1000: Sanitize linux_spi_read staging/wilc1000: Sanitize linux_spi_write staging/wilc1000: Remove unneeded imports staging/wilc1000: Remove unneeded USE_SPI_DMA staging/wilc1000: Remove unneeded forward declaration drivers/staging/wilc1000/linux_wlan_spi.c | 353 +++--- 1 file changed, 122 insertions(+), 231 deletions(-) -- 2.5.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/7] staging/wilc1000: Introduce linux_spi_msg_init
Since all spi read and write functions were basically preparing the same data, moving the preparation into a function decreased line count a lot. This also resolves the following problems: Zeroing the message ourselves is not needed, as spi_message_init() already does that for us. The comment on struct spi_transfer states that setting rx or tx to NULL will result in sending zeroes or discarding read data. Therefore we don't have to allocate an empty buffer if we only do one way transfer. Returning -ENOMEM on failed allocation would not have resulted in error catching but success in the callee. Thats because of the strange expected return values. Signed-off-by: Janosch Frank --- drivers/staging/wilc1000/linux_wlan_spi.c | 183 -- 1 file changed, 47 insertions(+), 136 deletions(-) diff --git a/drivers/staging/wilc1000/linux_wlan_spi.c b/drivers/staging/wilc1000/linux_wlan_spi.c index 039d061..72230d2 100644 --- a/drivers/staging/wilc1000/linux_wlan_spi.c +++ b/drivers/staging/wilc1000/linux_wlan_spi.c @@ -110,6 +110,25 @@ int linux_spi_init(void *vp) return ret; } +static void linux_spi_msg_init(struct spi_message *msg, struct spi_transfer *tr, + u32 len, u8 *tx, u8 *rx) +{ + spi_message_init(msg); + memset(tr, 0, sizeof(*tr)); + + msg->spi = wilc_spi_dev; + msg->is_dma_mapped = USE_SPI_DMA; + + tr->tx_buf = tx; + tr->rx_buf = rx; + + tr->len = len; + tr->speed_hz = SPEED; + tr->bits_per_word = 8; + + spi_message_add_tail(tr, msg); +} + #if defined(PLAT_WMS8304) #define TXRX_PHASE_SIZE (4096) #endif @@ -119,35 +138,20 @@ int linux_spi_init(void *vp) int linux_spi_write(u8 *b, u32 len) { int ret; + struct spi_message msg; + struct spi_transfer tr; if (len > 0 && b != NULL) { int i = 0; int blk = len / TXRX_PHASE_SIZE; int remainder = len % TXRX_PHASE_SIZE; - char *r_buffer = kzalloc(TXRX_PHASE_SIZE, GFP_KERNEL); - if (!r_buffer) - return -ENOMEM; - if (blk) { while (i < blk) { - struct spi_message msg; - struct spi_transfer tr = { - .tx_buf = b + (i * TXRX_PHASE_SIZE), - .len = TXRX_PHASE_SIZE, - .speed_hz = SPEED, - .bits_per_word = 8, - .delay_usecs = 0, - }; - - tr.rx_buf = r_buffer; - - memset(&msg, 0, sizeof(msg)); - spi_message_init(&msg); - msg.spi = wilc_spi_dev; - msg.is_dma_mapped = USE_SPI_DMA; - - spi_message_add_tail(&tr, &msg); + linux_spi_msg_init(&msg, &tr, TXRX_PHASE_SIZE, + b + (i * TXRX_PHASE_SIZE), + NULL); + ret = spi_sync(wilc_spi_dev, &msg); if (ret < 0) { PRINT_ER("SPI transaction failed\n"); @@ -157,28 +161,15 @@ int linux_spi_write(u8 *b, u32 len) } } if (remainder) { - struct spi_message msg; - struct spi_transfer tr = { - .tx_buf = b + (blk * TXRX_PHASE_SIZE), - .len = remainder, - .speed_hz = SPEED, - .bits_per_word = 8, - .delay_usecs = 0, - }; - tr.rx_buf = r_buffer; - - memset(&msg, 0, sizeof(msg)); - spi_message_init(&msg); - msg.spi = wilc_spi_dev; - msg.is_dma_mapped = USE_SPI_DMA; /* rachel */ - - spi_message_add_tail(&tr, &msg); + linux_spi_msg_init(&msg, &tr, remainder, + b + (blk * TXRX_PHASE_SIZE), + NULL); + ret = spi_sync(wilc_spi_dev, &msg); if (ret < 0) { PRINT_ER("SPI transaction failed\n"); } } - kfree(r_buffer); } else { PRINT_ER("can't write data with the follow
[PATCH 5/7] staging/wilc1000: Remove unneeded imports
The only external function is spi_sync from spi/spi.h, no external kernel variables are used. Signed-off-by: Janosch Frank --- drivers/staging/wilc1000/linux_wlan_spi.c | 9 - 1 file changed, 9 deletions(-) diff --git a/drivers/staging/wilc1000/linux_wlan_spi.c b/drivers/staging/wilc1000/linux_wlan_spi.c index 8ad5084..28d4cf0 100644 --- a/drivers/staging/wilc1000/linux_wlan_spi.c +++ b/drivers/staging/wilc1000/linux_wlan_spi.c @@ -1,12 +1,3 @@ -#include -#include -#include -#include -#include -#include -#include -#include -#include #include #include "linux_wlan_common.h" -- 2.5.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/7] staging/wilc1000: Remove unneeded USE_SPI_DMA
After zeroing the message, the value is zero anyway and we don't have to set it explicitly. Signed-off-by: Janosch Frank --- drivers/staging/wilc1000/linux_wlan_spi.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/staging/wilc1000/linux_wlan_spi.c b/drivers/staging/wilc1000/linux_wlan_spi.c index 28d4cf0..f4bdaab 100644 --- a/drivers/staging/wilc1000/linux_wlan_spi.c +++ b/drivers/staging/wilc1000/linux_wlan_spi.c @@ -3,8 +3,6 @@ #include "linux_wlan_common.h" #include "linux_wlan_spi.h" -#define USE_SPI_DMA 0 /* johnny add */ - #ifdef WILC_ASIC_A0 #if defined(PLAT_PANDA_ES_OMAP4460) #define MIN_SPEED 1200 @@ -108,7 +106,6 @@ static void linux_spi_msg_init(struct spi_message *msg, struct spi_transfer *tr, memset(tr, 0, sizeof(*tr)); msg->spi = wilc_spi_dev; - msg->is_dma_mapped = USE_SPI_DMA; tr->tx_buf = tx; tr->rx_buf = rx; -- 2.5.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 7/7] staging/wilc1000: Remove unneeded forward declaration
linux_spi_deinit is not used from within linux_wlan_spi.c, so the declaration is useless. Signed-off-by: Janosch Frank --- drivers/staging/wilc1000/linux_wlan_spi.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/staging/wilc1000/linux_wlan_spi.c b/drivers/staging/wilc1000/linux_wlan_spi.c index f4bdaab..0816f5a 100644 --- a/drivers/staging/wilc1000/linux_wlan_spi.c +++ b/drivers/staging/wilc1000/linux_wlan_spi.c @@ -29,9 +29,7 @@ #endif /* WILC_ASIC_A0 */ static u32 SPEED = MIN_SPEED; - struct spi_device *wilc_spi_dev; -void linux_spi_deinit(void *vp); static int __init wilc_bus_probe(struct spi_device *spi) { -- 2.5.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/7] staging/wilc1000: Sanitize linux_spi_write
Removed unneeded newlines. Moved variable definitions to the top. Improved control flow to get rid of indents. Replaced while with for. Added additional error message and improved existing ones. Signed-off-by: Janosch Frank --- drivers/staging/wilc1000/linux_wlan_spi.c | 87 ++- 1 file changed, 40 insertions(+), 47 deletions(-) diff --git a/drivers/staging/wilc1000/linux_wlan_spi.c b/drivers/staging/wilc1000/linux_wlan_spi.c index b0dd486..8ad5084 100644 --- a/drivers/staging/wilc1000/linux_wlan_spi.c +++ b/drivers/staging/wilc1000/linux_wlan_spi.c @@ -137,78 +137,71 @@ static void linux_spi_msg_init(struct spi_message *msg, struct spi_transfer *tr, int linux_spi_write(u8 *b, u32 len) { - int ret; + int ret, i; + int blk = len / TXRX_PHASE_SIZE; + int remainder = len % TXRX_PHASE_SIZE; struct spi_message msg; struct spi_transfer tr; - if (len > 0 && b != NULL) { - int i = 0; - int blk = len / TXRX_PHASE_SIZE; - int remainder = len % TXRX_PHASE_SIZE; - - if (blk) { - while (i < blk) { - linux_spi_msg_init(&msg, &tr, TXRX_PHASE_SIZE, - b + (i * TXRX_PHASE_SIZE), - NULL); + if (!len) { + PRINT_ER("Zero length write.\n"); + return 0; + } - ret = spi_sync(wilc_spi_dev, &msg); - if (ret < 0) { - PRINT_ER("SPI transaction failed\n"); - } - i++; + if (!b) { + PRINT_ER("Write buffer NULL.\n"); + return 0; + } - } - } - if (remainder) { - linux_spi_msg_init(&msg, &tr, remainder, - b + (blk * TXRX_PHASE_SIZE), - NULL); - - ret = spi_sync(wilc_spi_dev, &msg); - if (ret < 0) { - PRINT_ER("SPI transaction failed\n"); - } + for (i = 0; i < blk; i++) { + linux_spi_msg_init(&msg, &tr, TXRX_PHASE_SIZE, + b + (i * TXRX_PHASE_SIZE), + NULL); + ret = spi_sync(wilc_spi_dev, &msg); + if (ret < 0) { + PRINT_ER("SPI sync failed and returned %d.\n", ret); + return 0; } - } else { - PRINT_ER("can't write data with the following length: %d\n", len); - PRINT_ER("FAILED due to NULL buffer or ZERO length check the following length: %d\n", len); - ret = -1; + } + + if (remainder) { + linux_spi_msg_init(&msg, &tr, remainder, + b + (blk * TXRX_PHASE_SIZE), + NULL); + ret = spi_sync(wilc_spi_dev, &msg); + if (ret < 0) + PRINT_ER("SPI sync failed and returned %d.\n", ret); } /* change return value to match WILC interface */ (ret < 0) ? (ret = 0) : (ret = 1); - return ret; - } #else int linux_spi_write(u8 *b, u32 len) { - int ret; struct spi_message msg; struct spi_transfer tr; - if (len > 0 && b != NULL) { - linux_spi_msg_init(&msg, &tr, len, b, NULL); - - ret = spi_sync(wilc_spi_dev, &msg); - if (ret < 0) { - PRINT_ER("SPI transaction failed\n"); - } + if (!len) { + PRINT_ER("Zero length write.\n"); + return 0; + } - } else { - PRINT_ER("can't write data with the following length: %d\n", len); - PRINT_ER("FAILED due to NULL buffer or ZERO length check the following length: %d\n", len); - ret = -1; + if (!b) { + PRINT_ER("Write buffer NULL.\n"); + return 0; } + linux_spi_msg_init(&msg, &tr, len, b, NULL); + ret = spi_sync(wilc_spi_dev, &msg); + if (ret < 0) + PRINT_ER("SPI sync failed and returned %d.\n", ret); + /* change return value to match WILC interface */ (ret < 0) ? (ret = 0) : (ret = 1); - - return ret; } -- 2.5.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/7] staging/wilc1000: Remove unneeded imports
Am 10.01.2016 um 20:30 schrieb kbuild test robot: Hi Janosch, [auto build test ERROR on v4.4-rc8] [cannot apply to staging/staging-testing next-20160108] [if your patch is applied to the wrong git tree, please drop us a note to help improving the system] url: https://github.com/0day-ci/linux/commits/Janosch-Frank/staging-wilc1000-Refactor-spi-read-and-write-functions/20160111-022042 config: x86_64-randconfig-s0-01110305 (attached as .config) reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All error/warnings (new ones prefixed by >>): drivers/staging/wilc1000/linux_wlan_spi.c:60:1: warning: data definition has no type or storage class MODULE_DEVICE_TABLE(of, wilc1000_of_match); ^ drivers/staging/wilc1000/linux_wlan_spi.c:60:1: error: type defaults to 'int' in declaration of 'MODULE_DEVICE_TABLE' [-Werror=implicit-int] drivers/staging/wilc1000/linux_wlan_spi.c:60:1: warning: parameter names (without types) in function declaration cc1: some warnings being treated as errors vim +60 drivers/staging/wilc1000/linux_wlan_spi.c c5c77ba1 Johnny Kim 2015-05-11 54 c5c77ba1 Johnny Kim 2015-05-11 55 #ifdef CONFIG_OF c5c77ba1 Johnny Kim 2015-05-11 56 static const struct of_device_id wilc1000_of_match[] = { c5c77ba1 Johnny Kim 2015-05-11 57 { .compatible = "atmel,wilc_spi", }, c5c77ba1 Johnny Kim 2015-05-11 58 {} c5c77ba1 Johnny Kim 2015-05-11 59 }; c5c77ba1 Johnny Kim 2015-05-11 @60 MODULE_DEVICE_TABLE(of, wilc1000_of_match); c5c77ba1 Johnny Kim 2015-05-11 61 #endif c5c77ba1 Johnny Kim 2015-05-11 62 c5c77ba1 Johnny Kim 2015-05-11 63 struct spi_driver wilc_bus __refdata = { :: The code at line 60 was first introduced by commit :: c5c77ba18ea66aa05441c71e38473efb787705a4 staging: wilc1000: Add SDIO/SPI 802.11 driver :: TO: Johnny Kim :: CC: Greg Kroah-Hartman --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation > module.h should not have been removed, as it contains the macro. With the right .config I would have noticed that... Is a v2 needed/wanted? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 5/7] staging/wilc1000: Remove unneeded imports
Apart from spi.h and module.h, none of the imports are used anymore. Signed-off-by: Janosch Frank --- drivers/staging/wilc1000/linux_wlan_spi.c | 8 1 file changed, 8 deletions(-) diff --git a/drivers/staging/wilc1000/linux_wlan_spi.c b/drivers/staging/wilc1000/linux_wlan_spi.c index 8ad5084..c406572 100644 --- a/drivers/staging/wilc1000/linux_wlan_spi.c +++ b/drivers/staging/wilc1000/linux_wlan_spi.c @@ -1,12 +1,4 @@ #include -#include -#include -#include -#include -#include -#include -#include -#include #include #include "linux_wlan_common.h" -- 2.5.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 7/7] staging/wilc1000: Remove unneeded forward declaration
linux_spi_deinit is not used from within linux_wlan_spi.c, so the declaration is useless. Signed-off-by: Janosch Frank --- drivers/staging/wilc1000/linux_wlan_spi.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/staging/wilc1000/linux_wlan_spi.c b/drivers/staging/wilc1000/linux_wlan_spi.c index f1f5392..2f52aba 100644 --- a/drivers/staging/wilc1000/linux_wlan_spi.c +++ b/drivers/staging/wilc1000/linux_wlan_spi.c @@ -30,9 +30,7 @@ #endif /* WILC_ASIC_A0 */ static u32 SPEED = MIN_SPEED; - struct spi_device *wilc_spi_dev; -void linux_spi_deinit(void *vp); static int __init wilc_bus_probe(struct spi_device *spi) { -- 2.5.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 3/7] staging/wilc1000: Sanitize linux_spi_read
Removed unneeded newlines. Moved variable definitions to the top. Improved control flow to get rid of indents. Replaced while with for. Added additional error message and improved existing ones. Signed-off-by: Janosch Frank --- drivers/staging/wilc1000/linux_wlan_spi.c | 79 +++ 1 file changed, 39 insertions(+), 40 deletions(-) diff --git a/drivers/staging/wilc1000/linux_wlan_spi.c b/drivers/staging/wilc1000/linux_wlan_spi.c index 595e274..b0dd486 100644 --- a/drivers/staging/wilc1000/linux_wlan_spi.c +++ b/drivers/staging/wilc1000/linux_wlan_spi.c @@ -218,70 +218,69 @@ int linux_spi_write(u8 *b, u32 len) int linux_spi_read(u8 *rb, u32 rlen) { - int ret; + int ret, i; + int blk = rlen / TXRX_PHASE_SIZE; + int remainder = rlen % TXRX_PHASE_SIZE; struct spi_message msg; struct spi_transfer tr; - if (rlen > 0) { - int i = 0; - - int blk = rlen / TXRX_PHASE_SIZE; - int remainder = rlen % TXRX_PHASE_SIZE; + if (!rlen) { + PRINT_ER("Zero length read.\n"); + return 0; + } - if (blk) { - while (i < blk) { - linux_spi_msg_init(&msg, &tr, TXRX_PHASE_SIZE, - NULL, - rb + (i * TXRX_PHASE_SIZE)); + if (!rb) { + PRINT_ER("Read buffer NULL.\n"); + return 0; + } - ret = spi_sync(wilc_spi_dev, &msg); - if (ret < 0) { - PRINT_ER("SPI transaction failed\n"); - } - i++; - } + for (i = 0; i < blk; i++) { + linux_spi_msg_init(&msg, &tr, TXRX_PHASE_SIZE, NULL, + rb + (i * TXRX_PHASE_SIZE)); + ret = spi_sync(wilc_spi_dev, &msg); + if (ret < 0) { + PRINT_ER("SPI sync failed and returned %d.\n", ret); + return 0; } - if (remainder) { - linux_spi_msg_init(&msg, &tr, remainder, NULL, - rb + (blk * TXRX_PHASE_SIZE)); + } - ret = spi_sync(wilc_spi_dev, &msg); - if (ret < 0) { - PRINT_ER("SPI transaction failed\n"); - } - } - } else { - PRINT_ER("can't read data with the following length: %u\n", rlen); - ret = -1; + if (remainder) { + linux_spi_msg_init(&msg, &tr, remainder, NULL, + rb + (blk * TXRX_PHASE_SIZE)); + ret = spi_sync(wilc_spi_dev, &msg); + if (ret < 0) + PRINT_ER("SPI sync failed and returned %d.\n", ret); } + /* change return value to match WILC interface */ (ret < 0) ? (ret = 0) : (ret = 1); - return ret; } #else int linux_spi_read(u8 *rb, u32 rlen) { - int ret; struct spi_message msg; struct spi_transfer tr; - if (rlen > 0) { - linux_spi_msg_init(&msg, &tr, rlen, NULL, rb); + if (!rlen) { + PRINT_ER("Zero length read.\n"); + return 0; + } - ret = spi_sync(wilc_spi_dev, &msg); - if (ret < 0) { - PRINT_ER("SPI transaction failed\n"); - } - } else { - PRINT_ER("can't read data with the following length: %u\n", rlen); - ret = -1; + if (!rb) { + PRINT_ER("Read buffer NULL.\n"); + return 0; } + + linux_spi_msg_init(&msg, &tr, rlen, NULL, rb); + ret = spi_sync(wilc_spi_dev, &msg); + if (ret < 0) + PRINT_ER("SPI sync failed and returned %d.\n", ret); + /* change return value to match WILC interface */ (ret < 0) ? (ret = 0) : (ret = 1); - return ret; } -- 2.5.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/7] staging/wilc1000: Introduce linux_spi_msg_init
Since all spi read and write functions were basically preparing the same data, moving the preparation into a function decreased line count a lot. This also resolves the following problems: Zeroing the message ourselves is not needed, as spi_message_init() already does that for us. The comment on struct spi_transfer states that setting rx or tx to NULL will result in sending zeroes or discarding read data. Therefore we don't have to allocate an empty buffer if we only do one way transfer. Returning -ENOMEM on failed allocation would not have resulted in error catching but success in the callee. Thats because of the strange expected return values. Signed-off-by: Janosch Frank --- drivers/staging/wilc1000/linux_wlan_spi.c | 183 -- 1 file changed, 47 insertions(+), 136 deletions(-) diff --git a/drivers/staging/wilc1000/linux_wlan_spi.c b/drivers/staging/wilc1000/linux_wlan_spi.c index 039d061..72230d2 100644 --- a/drivers/staging/wilc1000/linux_wlan_spi.c +++ b/drivers/staging/wilc1000/linux_wlan_spi.c @@ -110,6 +110,25 @@ int linux_spi_init(void *vp) return ret; } +static void linux_spi_msg_init(struct spi_message *msg, struct spi_transfer *tr, + u32 len, u8 *tx, u8 *rx) +{ + spi_message_init(msg); + memset(tr, 0, sizeof(*tr)); + + msg->spi = wilc_spi_dev; + msg->is_dma_mapped = USE_SPI_DMA; + + tr->tx_buf = tx; + tr->rx_buf = rx; + + tr->len = len; + tr->speed_hz = SPEED; + tr->bits_per_word = 8; + + spi_message_add_tail(tr, msg); +} + #if defined(PLAT_WMS8304) #define TXRX_PHASE_SIZE (4096) #endif @@ -119,35 +138,20 @@ int linux_spi_init(void *vp) int linux_spi_write(u8 *b, u32 len) { int ret; + struct spi_message msg; + struct spi_transfer tr; if (len > 0 && b != NULL) { int i = 0; int blk = len / TXRX_PHASE_SIZE; int remainder = len % TXRX_PHASE_SIZE; - char *r_buffer = kzalloc(TXRX_PHASE_SIZE, GFP_KERNEL); - if (!r_buffer) - return -ENOMEM; - if (blk) { while (i < blk) { - struct spi_message msg; - struct spi_transfer tr = { - .tx_buf = b + (i * TXRX_PHASE_SIZE), - .len = TXRX_PHASE_SIZE, - .speed_hz = SPEED, - .bits_per_word = 8, - .delay_usecs = 0, - }; - - tr.rx_buf = r_buffer; - - memset(&msg, 0, sizeof(msg)); - spi_message_init(&msg); - msg.spi = wilc_spi_dev; - msg.is_dma_mapped = USE_SPI_DMA; - - spi_message_add_tail(&tr, &msg); + linux_spi_msg_init(&msg, &tr, TXRX_PHASE_SIZE, + b + (i * TXRX_PHASE_SIZE), + NULL); + ret = spi_sync(wilc_spi_dev, &msg); if (ret < 0) { PRINT_ER("SPI transaction failed\n"); @@ -157,28 +161,15 @@ int linux_spi_write(u8 *b, u32 len) } } if (remainder) { - struct spi_message msg; - struct spi_transfer tr = { - .tx_buf = b + (blk * TXRX_PHASE_SIZE), - .len = remainder, - .speed_hz = SPEED, - .bits_per_word = 8, - .delay_usecs = 0, - }; - tr.rx_buf = r_buffer; - - memset(&msg, 0, sizeof(msg)); - spi_message_init(&msg); - msg.spi = wilc_spi_dev; - msg.is_dma_mapped = USE_SPI_DMA; /* rachel */ - - spi_message_add_tail(&tr, &msg); + linux_spi_msg_init(&msg, &tr, remainder, + b + (blk * TXRX_PHASE_SIZE), + NULL); + ret = spi_sync(wilc_spi_dev, &msg); if (ret < 0) { PRINT_ER("SPI transaction failed\n"); } } - kfree(r_buffer); } else { PRINT_ER("can't write data with the follow
[PATCH v2 6/7] staging/wilc1000: Remove unneeded USE_SPI_DMA
After zeroing the message, the value is zero anyway and we don't have to set it explicitly. Signed-off-by: Janosch Frank --- drivers/staging/wilc1000/linux_wlan_spi.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/staging/wilc1000/linux_wlan_spi.c b/drivers/staging/wilc1000/linux_wlan_spi.c index c406572..f1f5392 100644 --- a/drivers/staging/wilc1000/linux_wlan_spi.c +++ b/drivers/staging/wilc1000/linux_wlan_spi.c @@ -4,8 +4,6 @@ #include "linux_wlan_common.h" #include "linux_wlan_spi.h" -#define USE_SPI_DMA 0 /* johnny add */ - #ifdef WILC_ASIC_A0 #if defined(PLAT_PANDA_ES_OMAP4460) #define MIN_SPEED 1200 @@ -109,7 +107,6 @@ static void linux_spi_msg_init(struct spi_message *msg, struct spi_transfer *tr, memset(tr, 0, sizeof(*tr)); msg->spi = wilc_spi_dev; - msg->is_dma_mapped = USE_SPI_DMA; tr->tx_buf = tx; tr->rx_buf = rx; -- 2.5.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/7] staging/wilc1000: Sanitize linux_spi_write_read
Removed unneeded newlines. Improved control flow to get rid of indents. Added additional error message and improved existing ones. Signed-off-by: Janosch Frank --- drivers/staging/wilc1000/linux_wlan_spi.c | 24 +--- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/drivers/staging/wilc1000/linux_wlan_spi.c b/drivers/staging/wilc1000/linux_wlan_spi.c index 72230d2..595e274 100644 --- a/drivers/staging/wilc1000/linux_wlan_spi.c +++ b/drivers/staging/wilc1000/linux_wlan_spi.c @@ -289,25 +289,27 @@ int linux_spi_read(u8 *rb, u32 rlen) int linux_spi_write_read(u8 *wb, u8 *rb, u32 rlen) { - int ret; struct spi_message msg; struct spi_transfer tr; - if (rlen > 0) { - linux_spi_msg_init(&msg, &tr, rlen, wb, rb); + if (!rlen) { + PRINT_ER("Zero length read/write.\n"); + return 0; + } - ret = spi_sync(wilc_spi_dev, &msg); - if (ret < 0) { - PRINT_ER("SPI transaction failed\n"); - } - } else { - PRINT_ER("can't read data with the following length: %u\n", rlen); - ret = -1; + if (!wb || !rb) { + PRINT_ER("Read or write buffer NULL.\n"); + return 0; } + + linux_spi_msg_init(&msg, &tr, rlen, wb, rb); + ret = spi_sync(wilc_spi_dev, &msg); + if (ret < 0) + PRINT_ER("SPI sync failed and returned %d.\n", ret); + /* change return value to match WILC interface */ (ret < 0) ? (ret = 0) : (ret = 1); - return ret; } -- 2.5.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 4/7] staging/wilc1000: Sanitize linux_spi_write
Removed unneeded newlines. Moved variable definitions to the top. Improved control flow to get rid of indents. Replaced while with for. Added additional error message and improved existing ones. Signed-off-by: Janosch Frank --- drivers/staging/wilc1000/linux_wlan_spi.c | 87 ++- 1 file changed, 40 insertions(+), 47 deletions(-) diff --git a/drivers/staging/wilc1000/linux_wlan_spi.c b/drivers/staging/wilc1000/linux_wlan_spi.c index b0dd486..8ad5084 100644 --- a/drivers/staging/wilc1000/linux_wlan_spi.c +++ b/drivers/staging/wilc1000/linux_wlan_spi.c @@ -137,78 +137,71 @@ static void linux_spi_msg_init(struct spi_message *msg, struct spi_transfer *tr, int linux_spi_write(u8 *b, u32 len) { - int ret; + int ret, i; + int blk = len / TXRX_PHASE_SIZE; + int remainder = len % TXRX_PHASE_SIZE; struct spi_message msg; struct spi_transfer tr; - if (len > 0 && b != NULL) { - int i = 0; - int blk = len / TXRX_PHASE_SIZE; - int remainder = len % TXRX_PHASE_SIZE; - - if (blk) { - while (i < blk) { - linux_spi_msg_init(&msg, &tr, TXRX_PHASE_SIZE, - b + (i * TXRX_PHASE_SIZE), - NULL); + if (!len) { + PRINT_ER("Zero length write.\n"); + return 0; + } - ret = spi_sync(wilc_spi_dev, &msg); - if (ret < 0) { - PRINT_ER("SPI transaction failed\n"); - } - i++; + if (!b) { + PRINT_ER("Write buffer NULL.\n"); + return 0; + } - } - } - if (remainder) { - linux_spi_msg_init(&msg, &tr, remainder, - b + (blk * TXRX_PHASE_SIZE), - NULL); - - ret = spi_sync(wilc_spi_dev, &msg); - if (ret < 0) { - PRINT_ER("SPI transaction failed\n"); - } + for (i = 0; i < blk; i++) { + linux_spi_msg_init(&msg, &tr, TXRX_PHASE_SIZE, + b + (i * TXRX_PHASE_SIZE), + NULL); + ret = spi_sync(wilc_spi_dev, &msg); + if (ret < 0) { + PRINT_ER("SPI sync failed and returned %d.\n", ret); + return 0; } - } else { - PRINT_ER("can't write data with the following length: %d\n", len); - PRINT_ER("FAILED due to NULL buffer or ZERO length check the following length: %d\n", len); - ret = -1; + } + + if (remainder) { + linux_spi_msg_init(&msg, &tr, remainder, + b + (blk * TXRX_PHASE_SIZE), + NULL); + ret = spi_sync(wilc_spi_dev, &msg); + if (ret < 0) + PRINT_ER("SPI sync failed and returned %d.\n", ret); } /* change return value to match WILC interface */ (ret < 0) ? (ret = 0) : (ret = 1); - return ret; - } #else int linux_spi_write(u8 *b, u32 len) { - int ret; struct spi_message msg; struct spi_transfer tr; - if (len > 0 && b != NULL) { - linux_spi_msg_init(&msg, &tr, len, b, NULL); - - ret = spi_sync(wilc_spi_dev, &msg); - if (ret < 0) { - PRINT_ER("SPI transaction failed\n"); - } + if (!len) { + PRINT_ER("Zero length write.\n"); + return 0; + } - } else { - PRINT_ER("can't write data with the following length: %d\n", len); - PRINT_ER("FAILED due to NULL buffer or ZERO length check the following length: %d\n", len); - ret = -1; + if (!b) { + PRINT_ER("Write buffer NULL.\n"); + return 0; } + linux_spi_msg_init(&msg, &tr, len, b, NULL); + ret = spi_sync(wilc_spi_dev, &msg); + if (ret < 0) + PRINT_ER("SPI sync failed and returned %d.\n", ret); + /* change return value to match WILC interface */ (ret < 0) ? (ret = 0) : (ret = 1); - - return ret; } -- 2.5.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 0/7] staging/wilc1000: Refactor spi read and write functions
The code for sending and receiving spi data had multiple style problems, was repetative and contained errors. This series simplifies the code and fixes some of the problems. v1 to v2: Readded module.h, as MODULE_DEVICE_TABLE depends on it. Janosch Frank (7): staging/wilc1000: Introduce linux_spi_msg_init staging/wilc1000: Sanitize linux_spi_write_read staging/wilc1000: Sanitize linux_spi_read staging/wilc1000: Sanitize linux_spi_write staging/wilc1000: Remove unneeded imports staging/wilc1000: Remove unneeded USE_SPI_DMA staging/wilc1000: Remove unneeded forward declaration drivers/staging/wilc1000/linux_wlan_spi.c | 352 +++--- 1 file changed, 122 insertions(+), 230 deletions(-) -- 2.5.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel