Re: [PATCH v2 1/7] staging/wilc1000: Introduce linux_spi_msg_init

[Sudip Mukherjee]

On Mon, Jan 11, 2016 at 09:13:31PM +0100, Janosch Frank wrote:
> 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 
> --

linux_wlan_spi.c has been deleted by:
523fc23f1179 ("staging: wilc1000: remove unused files")

regards
sudip
___
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

[Janosch Frank]

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(, 0, sizeof(msg));
-   spi_message_init();
-   msg.spi = wilc_spi_dev;
-   msg.is_dma_mapped = USE_SPI_DMA;
-
-   spi_message_add_tail(, );
+   linux_spi_msg_init(, , TXRX_PHASE_SIZE,
+  b + (i * TXRX_PHASE_SIZE),
+  NULL);
+
ret = spi_sync(wilc_spi_dev, );
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(, 0, sizeof(msg));
-   spi_message_init();
-   msg.spi = wilc_spi_dev;
-   msg.is_dma_mapped = USE_SPI_DMA;
/* rachel */
-
-   spi_message_add_tail(, );
+   linux_spi_msg_init(, , remainder,
+  b + (blk * TXRX_PHASE_SIZE),
+  NULL);
+
ret = spi_sync(wilc_spi_dev, );
if (ret < 0) {
PRINT_ER("SPI transaction failed\n");
}
}
-   kfree(r_buffer);
} 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);
@@ -198,35 +189,16 @@ int