Re: [PATCH] spi-atmel.c: fix DMA for bits_per_word > 8

[Nicolas Ferre <nicolas.ferre@xxxxxxxxx>]

David,

It seems that your email client is doing funny things with the source
code whitespace. Anyway, I can deal with this and re-format your patch:
no problem. For later patches I advice you to use "git format-patch"
command.

Still, see comments below...

Le 13/10/2015 00:32, David Mosberger a écrit :
> I need the patch below to make DMA work in spi-atmel.c when bits_per_word > 8.

A little bit more explanation would be good.

>   --david

I would need your "Signed-off-by:" tag.

> -- 
> eGauge Systems LLC, http://egauge.net/, 1.877-EGAUGE1, fax 720.545.9768
> 
> diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
> index 04e48e5..7bb361a 100644
> --- a/drivers/spi/spi-atmel.c
> +++ b/drivers/spi/spi-atmel.c
> @@ -711,6 +711,7 @@ static void atmel_spi_next_xfer_pio(struct
> spi_master *master,
>   * Submit next transfer for DMA.
>   */
>  static int atmel_spi_next_xfer_dma_submit(struct spi_master *master,
> +  struct spi_message *msg,
>   struct spi_transfer *xfer,
>   u32 *plen)
>  {
> @@ -721,7 +722,7 @@ static int atmel_spi_next_xfer_dma_submit(struct
> spi_master *master,
>   struct dma_async_tx_descriptor *txdesc;
>   struct dma_slave_config slave_config;
>   dma_cookie_t cookie;
> - u32 len = *plen;
> + u32 len = *plen, bpw;
> 
>   dev_vdbg(master->dev.parent, "atmel_spi_next_xfer_dma_submit\n");
> 
> @@ -758,7 +759,11 @@ static int atmel_spi_next_xfer_dma_submit(struct
> spi_master *master,
> 
>   *plen = len;
> 
> - if (atmel_spi_dma_slave_config(as, &slave_config, 8))
> + bpw = xfer->bits_per_word;
> + if (!bpw)

I assume that xfer->bits_per_word must be filled with the proper value.
So why do you need this test and de additional assignment below? Does it
come from a bad experience with the "xfer->bits_per_word" value being empty?

> + bpw = msg->spi->bits_per_word;
> +
> + if (atmel_spi_dma_slave_config(as, &slave_config, bpw))

I may have called bpw variable as "bits" like elsewhere in this driver:
it's a detail though.

>   goto err_exit;
> 
>   /* Send both scatterlists */
> @@ -1316,8 +1321,8 @@ static int atmel_spi_one_transfer(struct
> spi_master *master,
>   atmel_spi_pdc_next_xfer(master, msg, xfer);
>   } else if (atmel_spi_use_dma(as, xfer)) {
>   len = as->current_remaining_bytes;
> - ret = atmel_spi_next_xfer_dma_submit(master,
> - xfer, &len);
> + ret = atmel_spi_next_xfer_dma_submit(master, msg,
> +     xfer, &len);
>   if (ret) {
>   dev_err(&spi->dev,
>   "unable to use DMA, fallback to PIO\n");

Ok, the patch seems reasonable. So, can you please answer my main
question and give me the permission to add your "Signed-off-by:" tag or
re-send me a more standard patch...

Thanks a lot for your work on this driver David.
Best regards,
-- 
Nicolas Ferre
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html