linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH 1/2] dma: Add interface to calculate data transferred
  2013-10-11 21:42 ` [PATCH 1/2] dma: Add interface to calculate data transferred Youquan Song
@ 2013-10-11 10:22   ` Shevchenko, Andriy
  2013-10-11 13:33   ` Greg KH
  2013-10-11 21:42   ` [PATCH 2/2] dma: calculate the data tranferred by 8250 Youquan Song
  2 siblings, 0 replies; 18+ messages in thread
From: Shevchenko, Andriy @ 2013-10-11 10:22 UTC (permalink / raw)
  To: Song, Youquan
  Cc: Williams, Dan J, Koul, Vinod, gregkh, Westerberg, Mika,
	linux-kernel, Youquan Song

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2204 bytes --]

On Fri, 2013-10-11 at 17:42 -0400, Youquan Song wrote:
> Currently, the DMA channel calculates its data transferred only at network
> device driver. When other devices like UART or SPI etc, transfers data by DMA 
> mode, but it always shows 0 at /sys/class/dma/dma0chan*/bytes_transferred.
> 
> This patch add a new function which will calculate how many the data has been
> transferred after doing it by DMA mode. It can be used by other modules and
> also simplify current duplicated code.

Thanks for the patch. My comments below.

First of all, what is the point to have every device driver that uses
DMA to increment bytes_transferred value? It will show just amount of
data transferred through certain channel. How it could be used then?

> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -901,6 +901,23 @@ void dma_async_device_unregister(struct dma_device *device)
>  }
>  EXPORT_SYMBOL(dma_async_device_unregister);
>  
> +dma_cookie_t
> +dma_tx_submit_cal(struct dma_async_tx_descriptor *tx,
> +		struct dma_chan *chan, size_t len)

I think there is a better name for the function.
dmaengine_submit_and_count() for example?

> +{
> +
> +	dma_cookie_t cookie;

Above lines probably have to be exchanged.

> +	cookie = tx->tx_submit(tx);

And you may incorporate this line into above.

> +
> +	preempt_disable();
> +	__this_cpu_add(chan->local->bytes_transferred, len);
> +	__this_cpu_inc(chan->local->memcpy_count);
> +	preempt_enable();
> +
> +	return cookie;
> +

Redundant empty line.

> +}

-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] dma: calculate the data tranferred by 8250
  2013-10-11 21:42   ` [PATCH 2/2] dma: calculate the data tranferred by 8250 Youquan Song
@ 2013-10-11 13:32     ` Greg KH
  0 siblings, 0 replies; 18+ messages in thread
From: Greg KH @ 2013-10-11 13:32 UTC (permalink / raw)
  To: Youquan Song
  Cc: dan.j.williams, vinod.koul, andriy.shevchenko, mika.westerberg,
	linux-kernel, Youquan Song

On Fri, Oct 11, 2013 at 05:42:18PM -0400, Youquan Song wrote:
> When using UART transfers data by DMA mode, but it always shows 0 at 
> /sys/class/dma/dma0chan*/bytes_transferred.
> 
> Call the new function to calculate how many the data has been transferred
>  after doing it by DMA mode. 

How nice, you just leaked to userspace the size of passwords and other
sensitive things typed into a serial console :(

No, we can't do this, we fixed up this problem in other places we leaked
the transmition of serial data through proc and sysfs files, let's not
add new ones and go backwards in security.

Sorry, we can't do this, and I really doubt that it matters.

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] dma: Add interface to calculate data transferred
  2013-10-11 21:42 ` [PATCH 1/2] dma: Add interface to calculate data transferred Youquan Song
  2013-10-11 10:22   ` Shevchenko, Andriy
@ 2013-10-11 13:33   ` Greg KH
  2013-10-13 15:26     ` Vinod Koul
  2013-10-11 21:42   ` [PATCH 2/2] dma: calculate the data tranferred by 8250 Youquan Song
  2 siblings, 1 reply; 18+ messages in thread
From: Greg KH @ 2013-10-11 13:33 UTC (permalink / raw)
  To: Youquan Song
  Cc: dan.j.williams, vinod.koul, andriy.shevchenko, mika.westerberg,
	linux-kernel, Youquan Song

On Fri, Oct 11, 2013 at 05:42:17PM -0400, Youquan Song wrote:
> Currently, the DMA channel calculates its data transferred only at network
> device driver. When other devices like UART or SPI etc, transfers data by DMA 
> mode, but it always shows 0 at /sys/class/dma/dma0chan*/bytes_transferred.

Is that really a problem?  I have never heard anyone complaining about
it.  Where are the reports of this?

> This patch add a new function which will calculate how many the data has been
> transferred after doing it by DMA mode. It can be used by other modules and
> also simplify current duplicated code.
> 
> Signed-off-by: Youquan Song <youquan.song@intel.com>
> ---
>  drivers/dma/dmaengine.c   |   35 +++++++++++++++++++----------------
>  include/linux/dmaengine.h |    3 +++
>  2 files changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 9162ac8..4356a7e 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -901,6 +901,23 @@ void dma_async_device_unregister(struct dma_device *device)
>  }
>  EXPORT_SYMBOL(dma_async_device_unregister);
>  
> +dma_cookie_t
> +dma_tx_submit_cal(struct dma_async_tx_descriptor *tx,
> +		struct dma_chan *chan, size_t len)
> +{
> +
> +	dma_cookie_t cookie;
> +	cookie = tx->tx_submit(tx);
> +
> +	preempt_disable();
> +	__this_cpu_add(chan->local->bytes_transferred, len);
> +	__this_cpu_inc(chan->local->memcpy_count);
> +	preempt_enable();
> +
> +	return cookie;
> +
> +}

You create a function, yet no driver can use it as it's not exported, so
I guess you don't want anyone to use it :)

Anyway, I don't think this is a good idea, see my response to the serial
driver patch for why not.

sorry,

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread

* DMA: Calculate how many data transferred by DMA
@ 2013-10-11 21:42 Youquan Song
  2013-10-11 21:42 ` [PATCH 1/2] dma: Add interface to calculate data transferred Youquan Song
  0 siblings, 1 reply; 18+ messages in thread
From: Youquan Song @ 2013-10-11 21:42 UTC (permalink / raw)
  To: dan.j.williams, vinod.koul, gregkh
  Cc: andriy.shevchenko, mika.westerberg, linux-kernel, Youquan Song,
	Youquan Song

Currently, the DMA channel calculates its data transferred only at network
device driver. When other devices like UART or SPI etc, transfers data by DMA
mode, but it always shows 0 at /sys/class/dma/dma0chan*/bytes_transferred.
It will possibly mislead user that the DMA engine does not work.

This patch add a new function which will calculate how many the data has been
transferred after doing it by DMA mode. It can be used by other modules and
also simplify current duplicated code.

Add the interface when UART transfer data by Designware DMA engine. It will
calculate the data already tranferred in the DMA channel.

If the patch work, I will add the interface to other modules when needed.  


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/2] dma: Add interface to calculate data transferred
  2013-10-11 21:42 DMA: Calculate how many data transferred by DMA Youquan Song
@ 2013-10-11 21:42 ` Youquan Song
  2013-10-11 10:22   ` Shevchenko, Andriy
                     ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Youquan Song @ 2013-10-11 21:42 UTC (permalink / raw)
  To: dan.j.williams, vinod.koul, gregkh
  Cc: andriy.shevchenko, mika.westerberg, linux-kernel, Youquan Song,
	Youquan Song

Currently, the DMA channel calculates its data transferred only at network
device driver. When other devices like UART or SPI etc, transfers data by DMA 
mode, but it always shows 0 at /sys/class/dma/dma0chan*/bytes_transferred.

This patch add a new function which will calculate how many the data has been
transferred after doing it by DMA mode. It can be used by other modules and
also simplify current duplicated code.

Signed-off-by: Youquan Song <youquan.song@intel.com>
---
 drivers/dma/dmaengine.c   |   35 +++++++++++++++++++----------------
 include/linux/dmaengine.h |    3 +++
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 9162ac8..4356a7e 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -901,6 +901,23 @@ void dma_async_device_unregister(struct dma_device *device)
 }
 EXPORT_SYMBOL(dma_async_device_unregister);
 
+dma_cookie_t
+dma_tx_submit_cal(struct dma_async_tx_descriptor *tx,
+		struct dma_chan *chan, size_t len)
+{
+
+	dma_cookie_t cookie;
+	cookie = tx->tx_submit(tx);
+
+	preempt_disable();
+	__this_cpu_add(chan->local->bytes_transferred, len);
+	__this_cpu_inc(chan->local->memcpy_count);
+	preempt_enable();
+
+	return cookie;
+
+}
+
 /**
  * dma_async_memcpy_buf_to_buf - offloaded copy between virtual addresses
  * @chan: DMA channel to offload copy to
@@ -920,7 +937,6 @@ dma_async_memcpy_buf_to_buf(struct dma_chan *chan, void *dest,
 	struct dma_device *dev = chan->device;
 	struct dma_async_tx_descriptor *tx;
 	dma_addr_t dma_dest, dma_src;
-	dma_cookie_t cookie;
 	unsigned long flags;
 
 	dma_src = dma_map_single(dev->dev, src, len, DMA_TO_DEVICE);
@@ -937,14 +953,8 @@ dma_async_memcpy_buf_to_buf(struct dma_chan *chan, void *dest,
 	}
 
 	tx->callback = NULL;
-	cookie = tx->tx_submit(tx);
-
-	preempt_disable();
-	__this_cpu_add(chan->local->bytes_transferred, len);
-	__this_cpu_inc(chan->local->memcpy_count);
-	preempt_enable();
 
-	return cookie;
+	return dma_tx_submit_cal(tx, chan, len);
 }
 EXPORT_SYMBOL(dma_async_memcpy_buf_to_buf);
 
@@ -968,7 +978,6 @@ dma_async_memcpy_buf_to_pg(struct dma_chan *chan, struct page *page,
 	struct dma_device *dev = chan->device;
 	struct dma_async_tx_descriptor *tx;
 	dma_addr_t dma_dest, dma_src;
-	dma_cookie_t cookie;
 	unsigned long flags;
 
 	dma_src = dma_map_single(dev->dev, kdata, len, DMA_TO_DEVICE);
@@ -983,14 +992,8 @@ dma_async_memcpy_buf_to_pg(struct dma_chan *chan, struct page *page,
 	}
 
 	tx->callback = NULL;
-	cookie = tx->tx_submit(tx);
 
-	preempt_disable();
-	__this_cpu_add(chan->local->bytes_transferred, len);
-	__this_cpu_inc(chan->local->memcpy_count);
-	preempt_enable();
-
-	return cookie;
+	return dma_tx_submit_cal(tx, chan, len);
 }
 EXPORT_SYMBOL(dma_async_memcpy_buf_to_pg);
 
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 0bc7275..0025f8e 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -1084,4 +1084,7 @@ dma_cookie_t dma_memcpy_pg_to_iovec(struct dma_chan *chan, struct iovec *iov,
 	struct dma_pinned_list *pinned_list, struct page *page,
 	unsigned int offset, size_t len);
 
+dma_cookie_t dma_tx_submit_cal(struct dma_async_tx_descriptor *tx,
+	struct dma_chan *chan, size_t len);
+
 #endif /* DMAENGINE_H */
-- 
1.7.7.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/2] dma: calculate the data tranferred by 8250
  2013-10-11 21:42 ` [PATCH 1/2] dma: Add interface to calculate data transferred Youquan Song
  2013-10-11 10:22   ` Shevchenko, Andriy
  2013-10-11 13:33   ` Greg KH
@ 2013-10-11 21:42   ` Youquan Song
  2013-10-11 13:32     ` Greg KH
  2 siblings, 1 reply; 18+ messages in thread
From: Youquan Song @ 2013-10-11 21:42 UTC (permalink / raw)
  To: dan.j.williams, vinod.koul, gregkh
  Cc: andriy.shevchenko, mika.westerberg, linux-kernel, Youquan Song,
	Youquan Song

When using UART transfers data by DMA mode, but it always shows 0 at 
/sys/class/dma/dma0chan*/bytes_transferred.

Call the new function to calculate how many the data has been transferred
 after doing it by DMA mode. 

Signed-off-by: Youquan Song <youquan.song@intel.com>
---
 drivers/tty/serial/8250/8250_dma.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dma.c b/drivers/tty/serial/8250/8250_dma.c
index 7046769..b22ef80 100644
--- a/drivers/tty/serial/8250/8250_dma.c
+++ b/drivers/tty/serial/8250/8250_dma.c
@@ -83,7 +83,7 @@ int serial8250_tx_dma(struct uart_8250_port *p)
 	desc->callback = __dma_tx_complete;
 	desc->callback_param = p;
 
-	dma->tx_cookie = dmaengine_submit(desc);
+	dma->tx_cookie = dma_tx_submit_cal(desc, dma->txchan, dma->tx_size);
 
 	dma_sync_single_for_device(dma->txchan->device->dev, dma->tx_addr,
 				   UART_XMIT_SIZE, DMA_TO_DEVICE);
-- 
1.7.7.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] dma: Add interface to calculate data transferred
  2013-10-11 13:33   ` Greg KH
@ 2013-10-13 15:26     ` Vinod Koul
  2013-10-15 18:31       ` Youquan Song
  0 siblings, 1 reply; 18+ messages in thread
From: Vinod Koul @ 2013-10-13 15:26 UTC (permalink / raw)
  To: Greg KH
  Cc: Youquan Song, dan.j.williams, andriy.shevchenko, mika.westerberg,
	linux-kernel, Youquan Song

On Fri, Oct 11, 2013 at 06:33:43AM -0700, Greg KH wrote:
> On Fri, Oct 11, 2013 at 05:42:17PM -0400, Youquan Song wrote:
> > Currently, the DMA channel calculates its data transferred only at network
> > device driver. When other devices like UART or SPI etc, transfers data by DMA 
> > mode, but it always shows 0 at /sys/class/dma/dma0chan*/bytes_transferred.
> 
> Is that really a problem?  I have never heard anyone complaining about
> it.  Where are the reports of this?
Right, am not still getting the point on what is the problem that this series is
trying to fix..

~Vinod

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] dma: Add interface to calculate data transferred
  2013-10-15 18:31       ` Youquan Song
@ 2013-10-15 15:30         ` Greg KH
  2013-10-15 15:55         ` Dan Williams
  1 sibling, 0 replies; 18+ messages in thread
From: Greg KH @ 2013-10-15 15:30 UTC (permalink / raw)
  To: Youquan Song
  Cc: Vinod Koul, Youquan Song, dan.j.williams, andriy.shevchenko,
	mika.westerberg, linux-kernel

On Tue, Oct 15, 2013 at 02:31:42PM -0400, Youquan Song wrote:
> On Sun, Oct 13, 2013 at 08:56:33PM +0530, Vinod Koul wrote:
> > On Fri, Oct 11, 2013 at 06:33:43AM -0700, Greg KH wrote:
> > > On Fri, Oct 11, 2013 at 05:42:17PM -0400, Youquan Song wrote:
> > > > Currently, the DMA channel calculates its data transferred only at network
> > > > device driver. When other devices like UART or SPI etc, transfers data by DMA 
> > > > mode, but it always shows 0 at /sys/class/dma/dma0chan*/bytes_transferred.
> > > 
> > > Is that really a problem?  I have never heard anyone complaining about
> > > it.  Where are the reports of this?
> > Right, am not still getting the point on what is the problem that this series is
> > trying to fix..
> 
> The issue is that when I using UART to transfer data between to COMs
> which using Designware DMA controller channel. But I check the specific
> DMA channel by "cat /sys/class/dma/dma0chan3/bytes_transferred", but it
> should all "0". I have transferred data by UART port, why its DMA
> channel report "0" bytes transferred?  So I guess that it is possible
> the DMA device driver issue or the data does not use the Designware DMA channel
> fro transferred.  After check the code, I notice only when the DMA
> channel used by network device driver and it will record how much data has been
>  tranferred, why other device driver will not calculate it. Since DMA
> channel is used by other device driver, why only network is specific?  since it is
> common interface, the current /sys/class/dma/dma0chan*/bytes_transferred has
> much possibility to mislead the user.

If you are worried about the data transferred through a UART, then look
at the accounting variables for that device (in /proc/ ) and use them,
they should be a good enough hint to see that data is flowing, not the
dma data.

And again, I can't take this due to the security implications, we
already went down this path with the TTY bytes, why do you want us to
introduce the same issue again only to have to fix it again?

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] dma: Add interface to calculate data transferred
  2013-10-15 18:31       ` Youquan Song
  2013-10-15 15:30         ` Greg KH
@ 2013-10-15 15:55         ` Dan Williams
  2013-10-15 16:17           ` Greg KH
  2013-10-16  5:38           ` Vinod Koul
  1 sibling, 2 replies; 18+ messages in thread
From: Dan Williams @ 2013-10-15 15:55 UTC (permalink / raw)
  To: Youquan Song
  Cc: Vinod Koul, Greg KH, Youquan Song, andriy.shevchenko,
	mika.westerberg, linux-kernel

On Tue, Oct 15, 2013 at 11:31 AM, Youquan Song
<youquan.song@linux.intel.com> wrote:
> On Sun, Oct 13, 2013 at 08:56:33PM +0530, Vinod Koul wrote:
>> On Fri, Oct 11, 2013 at 06:33:43AM -0700, Greg KH wrote:
>> > On Fri, Oct 11, 2013 at 05:42:17PM -0400, Youquan Song wrote:
> The issue is that when I using UART to transfer data between to COMs
> which using Designware DMA controller channel. But I check the specific
> DMA channel by "cat /sys/class/dma/dma0chan3/bytes_transferred", but it
> should all "0". I have transferred data by UART port, why its DMA
> channel report "0" bytes transferred?  So I guess that it is possible
> the DMA device driver issue or the data does not use the Designware DMA channel
> fro transferred.  After check the code, I notice only when the DMA
> channel used by network device driver and it will record how much data has been
>  tranferred, why other device driver will not calculate it. Since DMA
> channel is used by other device driver, why only network is specific?  since it is
> common interface, the current /sys/class/dma/dma0chan*/bytes_transferred has
> much possibility to mislead the user.

Yes, and for that reason I think we should delete "
/sys/class/dma/dma0chan*/bytes_transferred" it really serves no useful
purpose besides "is my dma channel working" which can be determined by
other means.

--
Dan

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] dma: Add interface to calculate data transferred
  2013-10-15 15:55         ` Dan Williams
@ 2013-10-15 16:17           ` Greg KH
  2013-10-16  5:38           ` Vinod Koul
  1 sibling, 0 replies; 18+ messages in thread
From: Greg KH @ 2013-10-15 16:17 UTC (permalink / raw)
  To: Dan Williams
  Cc: Youquan Song, Vinod Koul, Youquan Song, andriy.shevchenko,
	mika.westerberg, linux-kernel

On Tue, Oct 15, 2013 at 08:55:20AM -0700, Dan Williams wrote:
> On Tue, Oct 15, 2013 at 11:31 AM, Youquan Song
> <youquan.song@linux.intel.com> wrote:
> > On Sun, Oct 13, 2013 at 08:56:33PM +0530, Vinod Koul wrote:
> >> On Fri, Oct 11, 2013 at 06:33:43AM -0700, Greg KH wrote:
> >> > On Fri, Oct 11, 2013 at 05:42:17PM -0400, Youquan Song wrote:
> > The issue is that when I using UART to transfer data between to COMs
> > which using Designware DMA controller channel. But I check the specific
> > DMA channel by "cat /sys/class/dma/dma0chan3/bytes_transferred", but it
> > should all "0". I have transferred data by UART port, why its DMA
> > channel report "0" bytes transferred?  So I guess that it is possible
> > the DMA device driver issue or the data does not use the Designware DMA channel
> > fro transferred.  After check the code, I notice only when the DMA
> > channel used by network device driver and it will record how much data has been
> >  tranferred, why other device driver will not calculate it. Since DMA
> > channel is used by other device driver, why only network is specific?  since it is
> > common interface, the current /sys/class/dma/dma0chan*/bytes_transferred has
> > much possibility to mislead the user.
> 
> Yes, and for that reason I think we should delete "
> /sys/class/dma/dma0chan*/bytes_transferred" it really serves no useful
> purpose besides "is my dma channel working" which can be determined by
> other means.

Sounds good to me, feel free to send a patch.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] dma: Add interface to calculate data transferred
  2013-10-13 15:26     ` Vinod Koul
@ 2013-10-15 18:31       ` Youquan Song
  2013-10-15 15:30         ` Greg KH
  2013-10-15 15:55         ` Dan Williams
  0 siblings, 2 replies; 18+ messages in thread
From: Youquan Song @ 2013-10-15 18:31 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Greg KH, Youquan Song, dan.j.williams, andriy.shevchenko,
	mika.westerberg, linux-kernel, Youquan Song

On Sun, Oct 13, 2013 at 08:56:33PM +0530, Vinod Koul wrote:
> On Fri, Oct 11, 2013 at 06:33:43AM -0700, Greg KH wrote:
> > On Fri, Oct 11, 2013 at 05:42:17PM -0400, Youquan Song wrote:
> > > Currently, the DMA channel calculates its data transferred only at network
> > > device driver. When other devices like UART or SPI etc, transfers data by DMA 
> > > mode, but it always shows 0 at /sys/class/dma/dma0chan*/bytes_transferred.
> > 
> > Is that really a problem?  I have never heard anyone complaining about
> > it.  Where are the reports of this?
> Right, am not still getting the point on what is the problem that this series is
> trying to fix..

The issue is that when I using UART to transfer data between to COMs
which using Designware DMA controller channel. But I check the specific
DMA channel by "cat /sys/class/dma/dma0chan3/bytes_transferred", but it
should all "0". I have transferred data by UART port, why its DMA
channel report "0" bytes transferred?  So I guess that it is possible
the DMA device driver issue or the data does not use the Designware DMA channel
fro transferred.  After check the code, I notice only when the DMA
channel used by network device driver and it will record how much data has been
 tranferred, why other device driver will not calculate it. Since DMA
channel is used by other device driver, why only network is specific?  since it is
common interface, the current /sys/class/dma/dma0chan*/bytes_transferred has
much possibility to mislead the user.


Thanks
-Youquan
 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] dma: Add interface to calculate data transferred
  2013-10-15 15:55         ` Dan Williams
  2013-10-15 16:17           ` Greg KH
@ 2013-10-16  5:38           ` Vinod Koul
  2013-10-16  8:36             ` Shevchenko, Andriy
  1 sibling, 1 reply; 18+ messages in thread
From: Vinod Koul @ 2013-10-16  5:38 UTC (permalink / raw)
  To: Dan Williams
  Cc: Youquan Song, Greg KH, Youquan Song, andriy.shevchenko,
	mika.westerberg, linux-kernel

On Tue, Oct 15, 2013 at 08:55:20AM -0700, Dan Williams wrote:
> On Tue, Oct 15, 2013 at 11:31 AM, Youquan Song
> <youquan.song@linux.intel.com> wrote:
> > On Sun, Oct 13, 2013 at 08:56:33PM +0530, Vinod Koul wrote:
> >> On Fri, Oct 11, 2013 at 06:33:43AM -0700, Greg KH wrote:
> >> > On Fri, Oct 11, 2013 at 05:42:17PM -0400, Youquan Song wrote:
> > The issue is that when I using UART to transfer data between to COMs
> > which using Designware DMA controller channel. But I check the specific
> > DMA channel by "cat /sys/class/dma/dma0chan3/bytes_transferred", but it
> > should all "0". I have transferred data by UART port, why its DMA
> > channel report "0" bytes transferred?  So I guess that it is possible
> > the DMA device driver issue or the data does not use the Designware DMA channel
> > fro transferred.  After check the code, I notice only when the DMA
> > channel used by network device driver and it will record how much data has been
> >  tranferred, why other device driver will not calculate it. Since DMA
> > channel is used by other device driver, why only network is specific?  since it is
> > common interface, the current /sys/class/dma/dma0chan*/bytes_transferred has
> > much possibility to mislead the user.
> 
> Yes, and for that reason I think we should delete "
> /sys/class/dma/dma0chan*/bytes_transferred" it really serves no useful
> purpose besides "is my dma channel working" which can be determined by
> other means.
Well am going to take it a bit further and ask you why do we need the
/sys/class/dma? I have never used it for slave work.

Does it get used anywhere in async_tx?

--
~Vinod

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] dma: Add interface to calculate data transferred
  2013-10-16  8:36             ` Shevchenko, Andriy
@ 2013-10-16  7:57               ` Vinod Koul
  2013-10-16  9:13                 ` Shevchenko, Andriy
  0 siblings, 1 reply; 18+ messages in thread
From: Vinod Koul @ 2013-10-16  7:57 UTC (permalink / raw)
  To: Shevchenko, Andriy
  Cc: Williams, Dan J, Youquan Song, Greg KH, Song, Youquan,
	Westerberg, Mika, linux-kernel

On Wed, Oct 16, 2013 at 02:06:30PM +0530, Shevchenko, Andriy wrote:
> On Wed, 2013-10-16 at 11:08 +0530, Vinod Koul wrote:
> > On Tue, Oct 15, 2013 at 08:55:20AM -0700, Dan Williams wrote:
> > > On Tue, Oct 15, 2013 at 11:31 AM, Youquan Song
> > > <youquan.song@linux.intel.com> wrote:
> > > > On Sun, Oct 13, 2013 at 08:56:33PM +0530, Vinod Koul wrote:
> > > >> On Fri, Oct 11, 2013 at 06:33:43AM -0700, Greg KH wrote:
> > > >> > On Fri, Oct 11, 2013 at 05:42:17PM -0400, Youquan Song wrote:
> > > > The issue is that when I using UART to transfer data between to COMs
> > > > which using Designware DMA controller channel. But I check the specific
> > > > DMA channel by "cat /sys/class/dma/dma0chan3/bytes_transferred", but it
> > > > should all "0". I have transferred data by UART port, why its DMA
> > > > channel report "0" bytes transferred?  So I guess that it is possible
> > > > the DMA device driver issue or the data does not use the Designware DMA channel
> > > > fro transferred.  After check the code, I notice only when the DMA
> > > > channel used by network device driver and it will record how much data has been
> > > >  tranferred, why other device driver will not calculate it. Since DMA
> > > > channel is used by other device driver, why only network is specific?  since it is
> > > > common interface, the current /sys/class/dma/dma0chan*/bytes_transferred has
> > > > much possibility to mislead the user.
> > > 
> > > Yes, and for that reason I think we should delete "
> > > /sys/class/dma/dma0chan*/bytes_transferred" it really serves no useful
> > > purpose besides "is my dma channel working" which can be determined by
> > > other means.
> > Well am going to take it a bit further and ask you why do we need the
> > /sys/class/dma? I have never used it for slave work.
> 
> How user (who, f.e., would like to run dmatest) will know names of the
> channels provided?
Ok dmatest requires this, I overlooked that part

> How could we see what channels of certain dma controller are requested /
> busy from userspace?
But do end user care or need to know about this?

--
~Vinod

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] dma: Add interface to calculate data transferred
  2013-10-16  5:38           ` Vinod Koul
@ 2013-10-16  8:36             ` Shevchenko, Andriy
  2013-10-16  7:57               ` Vinod Koul
  0 siblings, 1 reply; 18+ messages in thread
From: Shevchenko, Andriy @ 2013-10-16  8:36 UTC (permalink / raw)
  To: Koul, Vinod
  Cc: Williams, Dan J, Youquan Song, Greg KH, Song, Youquan,
	Westerberg, Mika, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2550 bytes --]

On Wed, 2013-10-16 at 11:08 +0530, Vinod Koul wrote:
> On Tue, Oct 15, 2013 at 08:55:20AM -0700, Dan Williams wrote:
> > On Tue, Oct 15, 2013 at 11:31 AM, Youquan Song
> > <youquan.song@linux.intel.com> wrote:
> > > On Sun, Oct 13, 2013 at 08:56:33PM +0530, Vinod Koul wrote:
> > >> On Fri, Oct 11, 2013 at 06:33:43AM -0700, Greg KH wrote:
> > >> > On Fri, Oct 11, 2013 at 05:42:17PM -0400, Youquan Song wrote:
> > > The issue is that when I using UART to transfer data between to COMs
> > > which using Designware DMA controller channel. But I check the specific
> > > DMA channel by "cat /sys/class/dma/dma0chan3/bytes_transferred", but it
> > > should all "0". I have transferred data by UART port, why its DMA
> > > channel report "0" bytes transferred?  So I guess that it is possible
> > > the DMA device driver issue or the data does not use the Designware DMA channel
> > > fro transferred.  After check the code, I notice only when the DMA
> > > channel used by network device driver and it will record how much data has been
> > >  tranferred, why other device driver will not calculate it. Since DMA
> > > channel is used by other device driver, why only network is specific?  since it is
> > > common interface, the current /sys/class/dma/dma0chan*/bytes_transferred has
> > > much possibility to mislead the user.
> > 
> > Yes, and for that reason I think we should delete "
> > /sys/class/dma/dma0chan*/bytes_transferred" it really serves no useful
> > purpose besides "is my dma channel working" which can be determined by
> > other means.
> Well am going to take it a bit further and ask you why do we need the
> /sys/class/dma? I have never used it for slave work.

How user (who, f.e., would like to run dmatest) will know names of the
channels provided?

How could we see what channels of certain dma controller are requested /
busy from userspace?

-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] dma: Add interface to calculate data transferred
  2013-10-16  7:57               ` Vinod Koul
@ 2013-10-16  9:13                 ` Shevchenko, Andriy
  2013-10-16 14:12                   ` Greg KH
  0 siblings, 1 reply; 18+ messages in thread
From: Shevchenko, Andriy @ 2013-10-16  9:13 UTC (permalink / raw)
  To: Koul, Vinod
  Cc: Williams, Dan J, Youquan Song, Greg KH, Song, Youquan,
	Westerberg, Mika, linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1398 bytes --]

On Wed, 2013-10-16 at 13:27 +0530, Vinod Koul wrote:
> On Wed, Oct 16, 2013 at 02:06:30PM +0530, Shevchenko, Andriy wrote:
> > On Wed, 2013-10-16 at 11:08 +0530, Vinod Koul wrote:

[]

> > > Well am going to take it a bit further and ask you why do we need the
> > > /sys/class/dma? I have never used it for slave work.
> > 
> > How user (who, f.e., would like to run dmatest) will know names of the
> > channels provided?
> Ok dmatest requires this, I overlooked that part
> 
> > How could we see what channels of certain dma controller are requested /
> > busy from userspace?
> But do end user care or need to know about this?

The idea is to have some test cases. So, end user probably doesn't
really need this.

-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] dma: Add interface to calculate data transferred
  2013-10-16  9:13                 ` Shevchenko, Andriy
@ 2013-10-16 14:12                   ` Greg KH
  2013-10-16 15:07                     ` Vinod Koul
  0 siblings, 1 reply; 18+ messages in thread
From: Greg KH @ 2013-10-16 14:12 UTC (permalink / raw)
  To: Shevchenko, Andriy
  Cc: Koul, Vinod, Williams, Dan J, Youquan Song, Song, Youquan,
	Westerberg, Mika, linux-kernel

On Wed, Oct 16, 2013 at 09:13:20AM +0000, Shevchenko, Andriy wrote:
> On Wed, 2013-10-16 at 13:27 +0530, Vinod Koul wrote:
> > On Wed, Oct 16, 2013 at 02:06:30PM +0530, Shevchenko, Andriy wrote:
> > > On Wed, 2013-10-16 at 11:08 +0530, Vinod Koul wrote:
> 
> []
> 
> > > > Well am going to take it a bit further and ask you why do we need the
> > > > /sys/class/dma? I have never used it for slave work.
> > > 
> > > How user (who, f.e., would like to run dmatest) will know names of the
> > > channels provided?
> > Ok dmatest requires this, I overlooked that part
> > 
> > > How could we see what channels of certain dma controller are requested /
> > > busy from userspace?
> > But do end user care or need to know about this?
> 
> The idea is to have some test cases. So, end user probably doesn't
> really need this.

So could this just be debugfs files instead?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] dma: Add interface to calculate data transferred
  2013-10-16 14:12                   ` Greg KH
@ 2013-10-16 15:07                     ` Vinod Koul
  2013-10-16 18:17                       ` Dan Williams
  0 siblings, 1 reply; 18+ messages in thread
From: Vinod Koul @ 2013-10-16 15:07 UTC (permalink / raw)
  To: Greg KH, Williams, Dan J
  Cc: Shevchenko, Andriy, Youquan Song, Song, Youquan, Westerberg,
	Mika, linux-kernel

On Wed, Oct 16, 2013 at 07:12:47AM -0700, Greg KH wrote:
> On Wed, Oct 16, 2013 at 09:13:20AM +0000, Shevchenko, Andriy wrote:
> > On Wed, 2013-10-16 at 13:27 +0530, Vinod Koul wrote:
> > > On Wed, Oct 16, 2013 at 02:06:30PM +0530, Shevchenko, Andriy wrote:
> > > > On Wed, 2013-10-16 at 11:08 +0530, Vinod Koul wrote:
> > 
> > []
> > 
> > > > > Well am going to take it a bit further and ask you why do we need the
> > > > > /sys/class/dma? I have never used it for slave work.
> > > > 
> > > > How user (who, f.e., would like to run dmatest) will know names of the
> > > > channels provided?
> > > Ok dmatest requires this, I overlooked that part
> > > 
> > > > How could we see what channels of certain dma controller are requested /
> > > > busy from userspace?
> > > But do end user care or need to know about this?
> > 
> > The idea is to have some test cases. So, end user probably doesn't
> > really need this.
> 
> So could this just be debugfs files instead?
Yes I was thinking on same lines and would agree with you on that. This _really_
doesn't need sysfs unless Dan some async usage dependent on it. While at it we
can review the fields and remove the ones which are not required...

Dan, let me know if you are okay with it, I will try to get that done over the
weekend...

--
~Vinod

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] dma: Add interface to calculate data transferred
  2013-10-16 15:07                     ` Vinod Koul
@ 2013-10-16 18:17                       ` Dan Williams
  0 siblings, 0 replies; 18+ messages in thread
From: Dan Williams @ 2013-10-16 18:17 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Greg KH, Shevchenko, Andriy, Youquan Song, Song, Youquan,
	Westerberg, Mika, linux-kernel, Dave Jiang

On Wed, Oct 16, 2013 at 8:07 AM, Vinod Koul <vinod.koul@intel.com> wrote:
> On Wed, Oct 16, 2013 at 07:12:47AM -0700, Greg KH wrote:
>> On Wed, Oct 16, 2013 at 09:13:20AM +0000, Shevchenko, Andriy wrote:
>> > On Wed, 2013-10-16 at 13:27 +0530, Vinod Koul wrote:
>> > > On Wed, Oct 16, 2013 at 02:06:30PM +0530, Shevchenko, Andriy wrote:
>> > > > On Wed, 2013-10-16 at 11:08 +0530, Vinod Koul wrote:
>> >
>> > []
>> >
>> > > > > Well am going to take it a bit further and ask you why do we need the
>> > > > > /sys/class/dma? I have never used it for slave work.
>> > > >
>> > > > How user (who, f.e., would like to run dmatest) will know names of the
>> > > > channels provided?
>> > > Ok dmatest requires this, I overlooked that part
>> > >
>> > > > How could we see what channels of certain dma controller are requested /
>> > > > busy from userspace?
>> > > But do end user care or need to know about this?
>> >
>> > The idea is to have some test cases. So, end user probably doesn't
>> > really need this.
>>
>> So could this just be debugfs files instead?
> Yes I was thinking on same lines and would agree with you on that. This _really_
> doesn't need sysfs unless Dan some async usage dependent on it. While at it we
> can review the fields and remove the ones which are not required...
>
> Dan, let me know if you are okay with it, I will try to get that done over the
> weekend...
>

Channel naming is the primary use case and several drivers rely on the
channel name in their print messages, some for naming resources, and
of course dmatest for identifying channels.  ioatdma uses it as a
parent device for allocating its per-channel 'quickdata' attributes to
advertise the capabilities and the ring_size which is useful for
determining channel loading and verifying the BIOS configuration.

An argument could be made to move the ioatdma attributes to debugfs,
but I don't see a clean/worthwhile way of ripping and replacing the
channel naming scheme for drivers.

--
Dan

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2013-10-16 18:17 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-11 21:42 DMA: Calculate how many data transferred by DMA Youquan Song
2013-10-11 21:42 ` [PATCH 1/2] dma: Add interface to calculate data transferred Youquan Song
2013-10-11 10:22   ` Shevchenko, Andriy
2013-10-11 13:33   ` Greg KH
2013-10-13 15:26     ` Vinod Koul
2013-10-15 18:31       ` Youquan Song
2013-10-15 15:30         ` Greg KH
2013-10-15 15:55         ` Dan Williams
2013-10-15 16:17           ` Greg KH
2013-10-16  5:38           ` Vinod Koul
2013-10-16  8:36             ` Shevchenko, Andriy
2013-10-16  7:57               ` Vinod Koul
2013-10-16  9:13                 ` Shevchenko, Andriy
2013-10-16 14:12                   ` Greg KH
2013-10-16 15:07                     ` Vinod Koul
2013-10-16 18:17                       ` Dan Williams
2013-10-11 21:42   ` [PATCH 2/2] dma: calculate the data tranferred by 8250 Youquan Song
2013-10-11 13:32     ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).