| Message ID | 20211207002453.3193737-1-davidm@egauge.net (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | dde02213fa64ac7f99eb65cd51f9e04812d8bc40 |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | wilc1000: Remove misleading USE_SPI_DMA macro | expand |
On 07/12/21 05:56, David Mosberger-Tang wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > The USE_SPI_DMA macro name suggests that it could be set to 1 to > control whether or not SPI DMA should be used. However, that's not > what it does. If set to 1, it'll set the SPI messages' > "is_dma_mapped" flag to true, even though the tx/rx buffers aren't > actually DMA mapped by the driver. In other words, setting this flag > to 1 will break the driver. > > Best to clean up this confusion by removing the macro altogether. > There is no need to explicitly initialize "is_dma_mapped" because the > message is cleared to zero anyhow, so "is_dma_mapped" is set to false > by default. > > Signed-off-by: David Mosberger-Tang <davidm@egauge.net> Thanks! Acked-by: Ajay Singh <ajay.kathat@microchip.com> > --- > drivers/net/wireless/microchip/wilc1000/spi.c | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/drivers/net/wireless/microchip/wilc1000/spi.c b/drivers/net/wireless/microchip/wilc1000/spi.c > index 640850f989dd..1856525b9834 100644 > --- a/drivers/net/wireless/microchip/wilc1000/spi.c > +++ b/drivers/net/wireless/microchip/wilc1000/spi.c > @@ -99,8 +99,6 @@ static int wilc_spi_reset(struct wilc *wilc); > #define DATA_PKT_LOG_SZ DATA_PKT_LOG_SZ_MAX > #define DATA_PKT_SZ (1 << DATA_PKT_LOG_SZ) > > -#define USE_SPI_DMA 0 > - > #define WILC_SPI_COMMAND_STAT_SUCCESS 0 > #define WILC_GET_RESP_HDR_START(h) (((h) >> 4) & 0xf) > > @@ -240,7 +238,6 @@ static int wilc_spi_tx(struct wilc *wilc, u8 *b, u32 len) > memset(&msg, 0, sizeof(msg)); > spi_message_init(&msg); > msg.spi = spi; > - msg.is_dma_mapped = USE_SPI_DMA; > spi_message_add_tail(&tr, &msg); > > ret = spi_sync(spi, &msg); > @@ -284,7 +281,6 @@ static int wilc_spi_rx(struct wilc *wilc, u8 *rb, u32 rlen) > memset(&msg, 0, sizeof(msg)); > spi_message_init(&msg); > msg.spi = spi; > - msg.is_dma_mapped = USE_SPI_DMA; > spi_message_add_tail(&tr, &msg); > > ret = spi_sync(spi, &msg); > @@ -323,7 +319,6 @@ static int wilc_spi_tx_rx(struct wilc *wilc, u8 *wb, u8 *rb, u32 rlen) > memset(&msg, 0, sizeof(msg)); > spi_message_init(&msg); > msg.spi = spi; > - msg.is_dma_mapped = USE_SPI_DMA; > > spi_message_add_tail(&tr, &msg); > ret = spi_sync(spi, &msg); > -- > 2.25.1 >
David Mosberger-Tang <davidm@egauge.net> wrote: > The USE_SPI_DMA macro name suggests that it could be set to 1 to > control whether or not SPI DMA should be used. However, that's not > what it does. If set to 1, it'll set the SPI messages' > "is_dma_mapped" flag to true, even though the tx/rx buffers aren't > actually DMA mapped by the driver. In other words, setting this flag > to 1 will break the driver. > > Best to clean up this confusion by removing the macro altogether. > There is no need to explicitly initialize "is_dma_mapped" because the > message is cleared to zero anyhow, so "is_dma_mapped" is set to false > by default. > > Signed-off-by: David Mosberger-Tang <davidm@egauge.net> > Acked-by: Ajay Singh <ajay.kathat@microchip.com> Patch applied to wireless-drivers-next.git, thanks. dde02213fa64 wilc1000: Remove misleading USE_SPI_DMA macro
diff --git a/drivers/net/wireless/microchip/wilc1000/spi.c b/drivers/net/wireless/microchip/wilc1000/spi.c index 640850f989dd..1856525b9834 100644 --- a/drivers/net/wireless/microchip/wilc1000/spi.c +++ b/drivers/net/wireless/microchip/wilc1000/spi.c @@ -99,8 +99,6 @@ static int wilc_spi_reset(struct wilc *wilc); #define DATA_PKT_LOG_SZ DATA_PKT_LOG_SZ_MAX #define DATA_PKT_SZ (1 << DATA_PKT_LOG_SZ) -#define USE_SPI_DMA 0 - #define WILC_SPI_COMMAND_STAT_SUCCESS 0 #define WILC_GET_RESP_HDR_START(h) (((h) >> 4) & 0xf) @@ -240,7 +238,6 @@ static int wilc_spi_tx(struct wilc *wilc, u8 *b, u32 len) memset(&msg, 0, sizeof(msg)); spi_message_init(&msg); msg.spi = spi; - msg.is_dma_mapped = USE_SPI_DMA; spi_message_add_tail(&tr, &msg); ret = spi_sync(spi, &msg); @@ -284,7 +281,6 @@ static int wilc_spi_rx(struct wilc *wilc, u8 *rb, u32 rlen) memset(&msg, 0, sizeof(msg)); spi_message_init(&msg); msg.spi = spi; - msg.is_dma_mapped = USE_SPI_DMA; spi_message_add_tail(&tr, &msg); ret = spi_sync(spi, &msg); @@ -323,7 +319,6 @@ static int wilc_spi_tx_rx(struct wilc *wilc, u8 *wb, u8 *rb, u32 rlen) memset(&msg, 0, sizeof(msg)); spi_message_init(&msg); msg.spi = spi; - msg.is_dma_mapped = USE_SPI_DMA; spi_message_add_tail(&tr, &msg); ret = spi_sync(spi, &msg);
The USE_SPI_DMA macro name suggests that it could be set to 1 to control whether or not SPI DMA should be used. However, that's not what it does. If set to 1, it'll set the SPI messages' "is_dma_mapped" flag to true, even though the tx/rx buffers aren't actually DMA mapped by the driver. In other words, setting this flag to 1 will break the driver. Best to clean up this confusion by removing the macro altogether. There is no need to explicitly initialize "is_dma_mapped" because the message is cleared to zero anyhow, so "is_dma_mapped" is set to false by default. Signed-off-by: David Mosberger-Tang <davidm@egauge.net> --- drivers/net/wireless/microchip/wilc1000/spi.c | 5 ----- 1 file changed, 5 deletions(-)