[PATCH 0/2] spi: atmel: Clean usage of DMA

[PATCH 0/2] spi: atmel: Clean usage of DMA

[Tudor Ambarus]

Cleaning patches done when reading DMA code. No functional change
expected. Tested on sama5d2_xplained with at25df321a.

Tudor Ambarus (2):
  spi: atmel: Drop slave_config argument in atmel_spi_dma_slave_config()
  spi: atmel: Remove setting of deprecated member of struct
    dma_slave_config

 drivers/spi/spi-atmel.c | 36 +++++++++++++++---------------------
 1 file changed, 15 insertions(+), 21 deletions(-)

-- 
2.25.1

[PATCH 1/2] spi: atmel: Drop slave_config argument in atmel_spi_dma_slave_config()

[Tudor Ambarus]

The callers passed a pointer to slave_config as an argument of
atmel_spi_dma_slave_config(), but they did not use it afterwards.
Use instead a local variable in atmel_spi_dma_slave_config(), and
stop passing arguments that are not needed in the callers.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/spi/spi-atmel.c | 38 +++++++++++++++++---------------------
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index f872cf196c2f..22c7239aff75 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -433,26 +433,25 @@ static bool atmel_spi_can_dma(struct spi_master *master,
 
 }
 
-static int atmel_spi_dma_slave_config(struct atmel_spi *as,
-				struct dma_slave_config *slave_config,
-				u8 bits_per_word)
+static int atmel_spi_dma_slave_config(struct atmel_spi *as, u8 bits_per_word)
 {
 	struct spi_master *master = platform_get_drvdata(as->pdev);
+	struct dma_slave_config	slave_config;
 	int err = 0;
 
 	if (bits_per_word > 8) {
-		slave_config->dst_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
-		slave_config->src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+		slave_config.dst_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+		slave_config.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
 	} else {
-		slave_config->dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
-		slave_config->src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+		slave_config.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+		slave_config.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
 	}
 
-	slave_config->dst_addr = (dma_addr_t)as->phybase + SPI_TDR;
-	slave_config->src_addr = (dma_addr_t)as->phybase + SPI_RDR;
-	slave_config->src_maxburst = 1;
-	slave_config->dst_maxburst = 1;
-	slave_config->device_fc = false;
+	slave_config.dst_addr = (dma_addr_t)as->phybase + SPI_TDR;
+	slave_config.src_addr = (dma_addr_t)as->phybase + SPI_RDR;
+	slave_config.src_maxburst = 1;
+	slave_config.dst_maxburst = 1;
+	slave_config.device_fc = false;
 
 	/*
 	 * This driver uses fixed peripheral select mode (PS bit set to '0' in
@@ -468,8 +467,8 @@ static int atmel_spi_dma_slave_config(struct atmel_spi *as,
 	 * So we'd rather write only one data at the time. Hence the transmit
 	 * path works the same whether FIFOs are available (and enabled) or not.
 	 */
-	slave_config->direction = DMA_MEM_TO_DEV;
-	if (dmaengine_slave_config(master->dma_tx, slave_config)) {
+	slave_config.direction = DMA_MEM_TO_DEV;
+	if (dmaengine_slave_config(master->dma_tx, &slave_config)) {
 		dev_err(&as->pdev->dev,
 			"failed to configure tx dma channel\n");
 		err = -EINVAL;
@@ -483,8 +482,8 @@ static int atmel_spi_dma_slave_config(struct atmel_spi *as,
 	 * So the receive path works the same whether FIFOs are available (and
 	 * enabled) or not.
 	 */
-	slave_config->direction = DMA_DEV_TO_MEM;
-	if (dmaengine_slave_config(master->dma_rx, slave_config)) {
+	slave_config.direction = DMA_DEV_TO_MEM;
+	if (dmaengine_slave_config(master->dma_rx, &slave_config)) {
 		dev_err(&as->pdev->dev,
 			"failed to configure rx dma channel\n");
 		err = -EINVAL;
@@ -496,7 +495,6 @@ static int atmel_spi_dma_slave_config(struct atmel_spi *as,
 static int atmel_spi_configure_dma(struct spi_master *master,
 				   struct atmel_spi *as)
 {
-	struct dma_slave_config	slave_config;
 	struct device *dev = &as->pdev->dev;
 	int err;
 
@@ -518,7 +516,7 @@ static int atmel_spi_configure_dma(struct spi_master *master,
 		goto error;
 	}
 
-	err = atmel_spi_dma_slave_config(as, &slave_config, 8);
+	err = atmel_spi_dma_slave_config(as, 8);
 	if (err)
 		goto error;
 
@@ -700,7 +698,6 @@ static int atmel_spi_next_xfer_dma_submit(struct spi_master *master,
 	struct dma_chan		*txchan = master->dma_tx;
 	struct dma_async_tx_descriptor *rxdesc;
 	struct dma_async_tx_descriptor *txdesc;
-	struct dma_slave_config	slave_config;
 	dma_cookie_t		cookie;
 
 	dev_vdbg(master->dev.parent, "atmel_spi_next_xfer_dma_submit\n");
@@ -712,8 +709,7 @@ static int atmel_spi_next_xfer_dma_submit(struct spi_master *master,
 
 	*plen = xfer->len;
 
-	if (atmel_spi_dma_slave_config(as, &slave_config,
-				       xfer->bits_per_word))
+	if (atmel_spi_dma_slave_config(as, xfer->bits_per_word))
 		goto err_exit;
 
 	/* Send both scatterlists */
-- 
2.25.1

[PATCH 2/2] spi: atmel: Remove setting of deprecated member of struct dma_slave_config

[Tudor Ambarus]

The 'direction' member of 'struct dma_slave_config' is deprecated.
Instead, drivers should use the direction argument to the
device_prep_slave_sg and device_prep_dma_cyclic functions or the
dir field in the dma_interleaved_template structure.
spi-atmel uses the direction argument to dmaengine_prep_slave_sg.
slave_config.direction is not used in neither of the DMA controller
drivers (at_h/xdmac) that spi-atmel is using, we can just remove the
setting of slave_config.direction and live with whatever stack value
is there.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/spi/spi-atmel.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index 22c7239aff75..f6626be4d4f7 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -467,7 +467,6 @@ static int atmel_spi_dma_slave_config(struct atmel_spi *as, u8 bits_per_word)
 	 * So we'd rather write only one data at the time. Hence the transmit
 	 * path works the same whether FIFOs are available (and enabled) or not.
 	 */
-	slave_config.direction = DMA_MEM_TO_DEV;
 	if (dmaengine_slave_config(master->dma_tx, &slave_config)) {
 		dev_err(&as->pdev->dev,
 			"failed to configure tx dma channel\n");
@@ -482,7 +481,6 @@ static int atmel_spi_dma_slave_config(struct atmel_spi *as, u8 bits_per_word)
 	 * So the receive path works the same whether FIFOs are available (and
 	 * enabled) or not.
 	 */
-	slave_config.direction = DMA_DEV_TO_MEM;
 	if (dmaengine_slave_config(master->dma_rx, &slave_config)) {
 		dev_err(&as->pdev->dev,
 			"failed to configure rx dma channel\n");
-- 
2.25.1

Re: [PATCH 0/2] spi: atmel: Clean usage of DMA

[Mark Brown]

On Thu, 25 Nov 2021 14:41:08 +0200, Tudor Ambarus wrote:
> Cleaning patches done when reading DMA code. No functional change
> expected. Tested on sama5d2_xplained with at25df321a.
> 
> Tudor Ambarus (2):
>   spi: atmel: Drop slave_config argument in atmel_spi_dma_slave_config()
>   spi: atmel: Remove setting of deprecated member of struct
>     dma_slave_config
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/2] spi: atmel: Drop slave_config argument in atmel_spi_dma_slave_config()
      commit: c1b00674aab0c6950970e52c0f059756064a9e8c
[2/2] spi: atmel: Remove setting of deprecated member of struct dma_slave_config
      commit: f44a29ceb99fc99832ee1d55d7fe9c4dbf594660

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark