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

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

[David Mosberger]

I need the patch below to make DMA work in spi-atmel.c when bits_per_word > 8.

  --david
-- 
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)
+ bpw = msg->spi->bits_per_word;
+
+ if (atmel_spi_dma_slave_config(as, &slave_config, bpw))
  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");

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

[Nicolas Ferre]

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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Nicolas Ferre]

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

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 548 bytes --]

On Mon, Oct 19, 2015 at 06:01:38PM +0200, Nicolas Ferre wrote:

> > - 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.

Please also remember to CC subsysetm maintainers on patches...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

[Mark Brown]

On Mon, Oct 19, 2015 at 06:01:38PM +0200, Nicolas Ferre wrote:

> > - 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.

Please also remember to CC subsysetm maintainers on patches...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151019/e603fa80/attachment.sig>

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

[David Mosberger]

Nicolas,

Thanks for your comments.

On Mon, Oct 19, 2015 at 10:01 AM, Nicolas Ferre <nicolas.ferre-AIFe0yeh4nA@public.gmane.orgm> wrote:
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.

Yeah, I should finally give up any hope that Gmail will get fixed to
not mangle tabs etc...

> - 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?

Sorry, that was a mistake.  I forgot that spi.c:__spi_validate() takes
care of setting the transfer's bits_per_word if it's not non-zero
already.

> 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...

I just sent an updated patch.

> Thanks a lot for your work on this driver David.

Oh, you're welcome!

  --david

On Mon, Oct 19, 2015 at 10:01 AM, Nicolas Ferre <nicolas.ferre-AIFe0yeh4nA@public.gmane.orgm> wrote:
> 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



-- 
eGauge Systems LLC, http://egauge.net/, 1.877-EGAUGE1, fax 720.545.9768
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[David Mosberger]

Nicolas,

Thanks for your comments.

On Mon, Oct 19, 2015 at 10:01 AM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
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.

Yeah, I should finally give up any hope that Gmail will get fixed to
not mangle tabs etc...

> - 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?

Sorry, that was a mistake.  I forgot that spi.c:__spi_validate() takes
care of setting the transfer's bits_per_word if it's not non-zero
already.

> 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...

I just sent an updated patch.

> Thanks a lot for your work on this driver David.

Oh, you're welcome!

  --david

On Mon, Oct 19, 2015 at 10:01 AM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
> 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



-- 
eGauge Systems LLC, http://egauge.net/, 1.877-EGAUGE1, fax 720.545.9768