Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Peter Ujfalusi]

Hi Radhey,

On 2018-05-30 20:29, Radhey Shyam Pandey wrote:
>> In couple of days I can update the metadata patches I have atm and send
>> as RFC.
>>
>> Is there anything from your side I should take into account when doing that?
> I think a generic interface to attach/share metadata buffer b/w client and the
> dmaengine driver is good enough. Is metadata patchset (early version) 
> available in TI external repos? 

I don't have it in public repository, but now that the TRM is public I
can start preparing things for upstream.

I have attached the patch I ended up with, but I need to add the
documentation part.

Since the 'metadata' is part of the DMA descriptor itself I thought that
it might be better to reflect that -> the metadata_ops is part of the
dma_async_tx_descriptor struct.

DMA drivers can initialize it when it is supported by the channel or
setup. In my case it is optional, many peripherals did not use it at all.

I have two modes to deal with the metadata:
1. attach mode
Client drivers are giving a buffer and a size to the DMA driver and in
case of TX the data is copied to the descriptor's metadata part. In case
of RX when the transfer is completed the DMA driver will copy the data
from the DMA descriptor to the client provided buffer.
Here we need one memcpy for each descriptor.

2. pointer mode
If we have high throughput peripheral, the per descriptor memcpy can be
an obstacle for performance.

TX: The client dmaengine_desc_get_metadata_ptr() to get the pointer to
the metadata section of the descriptor, it will receive back the max
size and the currently used size (important for RX). This is done before
issue_pending().
The client can update the metadata directly and when it is done calls
the dmaengine_desc_set_metadata_len() to tell the DMA driver the size of
the metadata it has configured.

RX: in the DMA callback the client calls
dmaengine_desc_get_metadata_ptr() to get the pointer and the size of the
metadata we have received and can process the information w/o memcpy.

I think it might be better to rename these from metadata to client_data
or something. It is part of the DMA descriptor, passed along with the
DMA descriptor, but it is owned by the client driver.

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>From cdd04a5876d5e2b1e10b4e5456585958715fd3a7 Mon Sep 17 00:00:00 2001
From: Peter Ujfalusi 
Date: Fri, 20 Apr 2018 15:10:08 +0300
Subject: [PATCH] dmaengine: Add metadat_ops for dma_async_tx_descriptor

If the DMA supports per descriptor metadata it can implement the attach,
get_ptr/set_len callbacks.

Client drivers must only use either attach or get_ptr/set_len to avoid
miss configuration.

Wrappers are also added for the metadata_ops:
dmaengine_desc_attach_metadata()
dmaengine_desc_get_metadata_ptr()
dmaengine_desc_set_metadata_len()

Signed-off-by: Peter Ujfalusi 
---
 include/linux/dmaengine.h | 50 +++
 1 file changed, 50 insertions(+)

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 51fbb861e84b..ac42ace36aa3 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -491,6 +491,18 @@ struct dmaengine_unmap_data {
 	dma_addr_t addr[0];
 };
 
+struct dma_async_tx_descriptor;
+
+struct dma_descriptor_metadata_ops {
+	int (*attach)(struct dma_async_tx_descriptor *desc, void *data,
+		  size_t len);
+
+	void *(*get_ptr)(struct dma_async_tx_descriptor *desc,
+			 size_t *payload_len, size_t *max_len);
+	int (*set_len)(struct dma_async_tx_descriptor *desc,
+		   size_t payload_len);
+};
+
 /**
  * struct dma_async_tx_descriptor - async transaction descriptor
  * ---dma generic offload fields---
@@ -520,6 +532,7 @@ struct dma_async_tx_descriptor {
 	dma_async_tx_callback_result callback_result;
 	void *callback_param;
 	struct dmaengine_unmap_data *unmap;
+	struct dma_descriptor_metadata_ops *metadata_ops;
 #ifdef CONFIG_ASYNC_TX_ENABLE_CHANNEL_SWITCH
 	struct dma_async_tx_descriptor *next;
 	struct dma_async_tx_descriptor *parent;
@@ -932,6 +945,43 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_dma_memcpy(
 		len, flags);
 }
 
+static inline int dmaengine_desc_attach_metadata(
+		struct dma_async_tx_descriptor *desc, void *data, size_t len)
+{
+	if (!desc)
+		return 0;
+
+	if (!desc->metadata_ops || !desc->metadata_ops->attach)
+		return -ENOTSUPP;
+
+	return desc->metadata_ops->attach(desc, data, len);
+}
+
+static inline void *dmaengine_desc_get_metadata_ptr(
+		struct dma_async_tx_descriptor *desc, size_t *payload_len,
+		size_t *max_len)
+{
+	if (!desc)
+		return NULL;
+
+	if (!desc->metadata_ops || !desc->metadata_ops->get_ptr)
+		return ERR_PTR(-ENOTSUPP);
+
+	return desc->metadata_ops->get_ptr(desc, payload_len, max_len);
+}
+
+static inline int dmaengine_desc_set_metadata_len(
+		struct dma_async_tx_descriptor *desc, size_t payload_len)
+{
+	if (!desc)
+		return 0;
+
+	if 

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Peter Ujfalusi]

Hi Radhey,

On 2018-05-30 20:29, Radhey Shyam Pandey wrote:
>> In couple of days I can update the metadata patches I have atm and send
>> as RFC.
>>
>> Is there anything from your side I should take into account when doing that?
> I think a generic interface to attach/share metadata buffer b/w client and the
> dmaengine driver is good enough. Is metadata patchset (early version) 
> available in TI external repos? 

I don't have it in public repository, but now that the TRM is public I
can start preparing things for upstream.

I have attached the patch I ended up with, but I need to add the
documentation part.

Since the 'metadata' is part of the DMA descriptor itself I thought that
it might be better to reflect that -> the metadata_ops is part of the
dma_async_tx_descriptor struct.

DMA drivers can initialize it when it is supported by the channel or
setup. In my case it is optional, many peripherals did not use it at all.

I have two modes to deal with the metadata:
1. attach mode
Client drivers are giving a buffer and a size to the DMA driver and in
case of TX the data is copied to the descriptor's metadata part. In case
of RX when the transfer is completed the DMA driver will copy the data
from the DMA descriptor to the client provided buffer.
Here we need one memcpy for each descriptor.

2. pointer mode
If we have high throughput peripheral, the per descriptor memcpy can be
an obstacle for performance.

TX: The client dmaengine_desc_get_metadata_ptr() to get the pointer to
the metadata section of the descriptor, it will receive back the max
size and the currently used size (important for RX). This is done before
issue_pending().
The client can update the metadata directly and when it is done calls
the dmaengine_desc_set_metadata_len() to tell the DMA driver the size of
the metadata it has configured.

RX: in the DMA callback the client calls
dmaengine_desc_get_metadata_ptr() to get the pointer and the size of the
metadata we have received and can process the information w/o memcpy.

I think it might be better to rename these from metadata to client_data
or something. It is part of the DMA descriptor, passed along with the
DMA descriptor, but it is owned by the client driver.

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>From cdd04a5876d5e2b1e10b4e5456585958715fd3a7 Mon Sep 17 00:00:00 2001
From: Peter Ujfalusi 
Date: Fri, 20 Apr 2018 15:10:08 +0300
Subject: [PATCH] dmaengine: Add metadat_ops for dma_async_tx_descriptor

If the DMA supports per descriptor metadata it can implement the attach,
get_ptr/set_len callbacks.

Client drivers must only use either attach or get_ptr/set_len to avoid
miss configuration.

Wrappers are also added for the metadata_ops:
dmaengine_desc_attach_metadata()
dmaengine_desc_get_metadata_ptr()
dmaengine_desc_set_metadata_len()

Signed-off-by: Peter Ujfalusi 
---
 include/linux/dmaengine.h | 50 +++
 1 file changed, 50 insertions(+)

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 51fbb861e84b..ac42ace36aa3 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -491,6 +491,18 @@ struct dmaengine_unmap_data {
 	dma_addr_t addr[0];
 };
 
+struct dma_async_tx_descriptor;
+
+struct dma_descriptor_metadata_ops {
+	int (*attach)(struct dma_async_tx_descriptor *desc, void *data,
+		  size_t len);
+
+	void *(*get_ptr)(struct dma_async_tx_descriptor *desc,
+			 size_t *payload_len, size_t *max_len);
+	int (*set_len)(struct dma_async_tx_descriptor *desc,
+		   size_t payload_len);
+};
+
 /**
  * struct dma_async_tx_descriptor - async transaction descriptor
  * ---dma generic offload fields---
@@ -520,6 +532,7 @@ struct dma_async_tx_descriptor {
 	dma_async_tx_callback_result callback_result;
 	void *callback_param;
 	struct dmaengine_unmap_data *unmap;
+	struct dma_descriptor_metadata_ops *metadata_ops;
 #ifdef CONFIG_ASYNC_TX_ENABLE_CHANNEL_SWITCH
 	struct dma_async_tx_descriptor *next;
 	struct dma_async_tx_descriptor *parent;
@@ -932,6 +945,43 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_dma_memcpy(
 		len, flags);
 }
 
+static inline int dmaengine_desc_attach_metadata(
+		struct dma_async_tx_descriptor *desc, void *data, size_t len)
+{
+	if (!desc)
+		return 0;
+
+	if (!desc->metadata_ops || !desc->metadata_ops->attach)
+		return -ENOTSUPP;
+
+	return desc->metadata_ops->attach(desc, data, len);
+}
+
+static inline void *dmaengine_desc_get_metadata_ptr(
+		struct dma_async_tx_descriptor *desc, size_t *payload_len,
+		size_t *max_len)
+{
+	if (!desc)
+		return NULL;
+
+	if (!desc->metadata_ops || !desc->metadata_ops->get_ptr)
+		return ERR_PTR(-ENOTSUPP);
+
+	return desc->metadata_ops->get_ptr(desc, payload_len, max_len);
+}
+
+static inline int dmaengine_desc_set_metadata_len(
+		struct dma_async_tx_descriptor *desc, size_t payload_len)
+{
+	if (!desc)
+		return 0;
+
+	if 

RE: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Radhey Shyam Pandey]

Hi,

> -Original Message-
> From: Peter Ujfalusi [mailto:peter.ujfal...@ti.com]
> Sent: Tuesday, May 29, 2018 8:35 PM
> To: Radhey Shyam Pandey ; Vinod Koul
> 
> Cc: Lars-Peter Clausen ; michal.si...@xilinx.com; linux-
> ker...@vger.kernel.org; dmaeng...@vger.kernel.org;
> dan.j.willi...@intel.com; Appana Durga Kedareswara Rao
> ; linux-arm-ker...@lists.infradead.org
> Subject: Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words
> to netdev dma client
> 
> Hi,
> 
> On 2018-05-17 09:39, Radhey Shyam Pandey wrote:
> >> Well, let's see where this is going to go when I can send the patches
> >> for review.
> > Thanks all. @Peter: If we have metadata patchset ready may be good
> > to send an RFC?
> 
> Sorry for the delay, I got distracted by this:
> http://www.ti.com/lit/pdf/spruid7 Chapter 10.
> 
> I have given some tough to the metadata attach patches.
> In my case the 'metadata' is more like private data section within the
> DMA descriptor (10.1.2.2.1) which is used by the remote peripheral and
> the driver for the given peripheral and it is optional.
> 
> I liked the idea of treating it as metadata as it gives more generic API
> which can be adopted by other drivers if they need something similar.
> 
> Another issue I have with the attach metadata way is that it would
> require one memcpy to copy the data to the DMA descriptor and in high
> throughput case it is not acceptable.

I think memcpy is needed (alternative?) if dma engine doesn’t directly
update metadata buffers i.e in RX, we need to copy metadata from 
dma descriptor fields to client allocated metadata buffer (sideband/
metadata info is part of Buffer descriptor fields) 

memcpy(app_w, hw->app, sizeof(u32) * XILINX_DMA_NUM_APP_WORDS);

Descriptor Fields
Address Space   Offset  Name Description
20h  APP0   User Application Field 0
24h  APP1   User Application Field 1
...
30h APP5   User Application Field 5

> 
> For me probably a .get_private_area / .put_private_area like API would
> be desirable where I can give the pointer of the 'metadata' are (and
> size) to the user.
> 
> But these can co-exist in my opinion and DMA drivers can opt to
> implement none, either or both of the callbacks.
> 
> In couple of days I can update the metadata patches I have atm and send
> as RFC.
> 
> Is there anything from your side I should take into account when doing that?
I think a generic interface to attach/share metadata buffer b/w client and the
dmaengine driver is good enough. Is metadata patchset (early version) 
available in TI external repos? 

Thanks,
Radhey

> 
> - Péter
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

RE: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Radhey Shyam Pandey]

Hi,

> -Original Message-
> From: Peter Ujfalusi [mailto:peter.ujfal...@ti.com]
> Sent: Tuesday, May 29, 2018 8:35 PM
> To: Radhey Shyam Pandey ; Vinod Koul
> 
> Cc: Lars-Peter Clausen ; michal.si...@xilinx.com; linux-
> ker...@vger.kernel.org; dmaeng...@vger.kernel.org;
> dan.j.willi...@intel.com; Appana Durga Kedareswara Rao
> ; linux-arm-ker...@lists.infradead.org
> Subject: Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words
> to netdev dma client
> 
> Hi,
> 
> On 2018-05-17 09:39, Radhey Shyam Pandey wrote:
> >> Well, let's see where this is going to go when I can send the patches
> >> for review.
> > Thanks all. @Peter: If we have metadata patchset ready may be good
> > to send an RFC?
> 
> Sorry for the delay, I got distracted by this:
> http://www.ti.com/lit/pdf/spruid7 Chapter 10.
> 
> I have given some tough to the metadata attach patches.
> In my case the 'metadata' is more like private data section within the
> DMA descriptor (10.1.2.2.1) which is used by the remote peripheral and
> the driver for the given peripheral and it is optional.
> 
> I liked the idea of treating it as metadata as it gives more generic API
> which can be adopted by other drivers if they need something similar.
> 
> Another issue I have with the attach metadata way is that it would
> require one memcpy to copy the data to the DMA descriptor and in high
> throughput case it is not acceptable.

I think memcpy is needed (alternative?) if dma engine doesn’t directly
update metadata buffers i.e in RX, we need to copy metadata from 
dma descriptor fields to client allocated metadata buffer (sideband/
metadata info is part of Buffer descriptor fields) 

memcpy(app_w, hw->app, sizeof(u32) * XILINX_DMA_NUM_APP_WORDS);

Descriptor Fields
Address Space   Offset  Name Description
20h  APP0   User Application Field 0
24h  APP1   User Application Field 1
...
30h APP5   User Application Field 5

> 
> For me probably a .get_private_area / .put_private_area like API would
> be desirable where I can give the pointer of the 'metadata' are (and
> size) to the user.
> 
> But these can co-exist in my opinion and DMA drivers can opt to
> implement none, either or both of the callbacks.
> 
> In couple of days I can update the metadata patches I have atm and send
> as RFC.
> 
> Is there anything from your side I should take into account when doing that?
I think a generic interface to attach/share metadata buffer b/w client and the
dmaengine driver is good enough. Is metadata patchset (early version) 
available in TI external repos? 

Thanks,
Radhey

> 
> - Péter
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Peter Ujfalusi]

Hi,

On 2018-05-17 09:39, Radhey Shyam Pandey wrote:
>> Well, let's see where this is going to go when I can send the patches
>> for review.
> Thanks all. @Peter: If we have metadata patchset ready may be good
> to send an RFC?

Sorry for the delay, I got distracted by this:
http://www.ti.com/lit/pdf/spruid7 Chapter 10.

I have given some tough to the metadata attach patches.
In my case the 'metadata' is more like private data section within the
DMA descriptor (10.1.2.2.1) which is used by the remote peripheral and
the driver for the given peripheral and it is optional.

I liked the idea of treating it as metadata as it gives more generic API
which can be adopted by other drivers if they need something similar.

Another issue I have with the attach metadata way is that it would
require one memcpy to copy the data to the DMA descriptor and in high
throughput case it is not acceptable.

For me probably a .get_private_area / .put_private_area like API would
be desirable where I can give the pointer of the 'metadata' are (and
size) to the user.

But these can co-exist in my opinion and DMA drivers can opt to
implement none, either or both of the callbacks.

In couple of days I can update the metadata patches I have atm and send
as RFC.

Is there anything from your side I should take into account when doing that?

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Peter Ujfalusi]

Hi,

On 2018-05-17 09:39, Radhey Shyam Pandey wrote:
>> Well, let's see where this is going to go when I can send the patches
>> for review.
> Thanks all. @Peter: If we have metadata patchset ready may be good
> to send an RFC?

Sorry for the delay, I got distracted by this:
http://www.ti.com/lit/pdf/spruid7 Chapter 10.

I have given some tough to the metadata attach patches.
In my case the 'metadata' is more like private data section within the
DMA descriptor (10.1.2.2.1) which is used by the remote peripheral and
the driver for the given peripheral and it is optional.

I liked the idea of treating it as metadata as it gives more generic API
which can be adopted by other drivers if they need something similar.

Another issue I have with the attach metadata way is that it would
require one memcpy to copy the data to the DMA descriptor and in high
throughput case it is not acceptable.

For me probably a .get_private_area / .put_private_area like API would
be desirable where I can give the pointer of the 'metadata' are (and
size) to the user.

But these can co-exist in my opinion and DMA drivers can opt to
implement none, either or both of the callbacks.

In couple of days I can update the metadata patches I have atm and send
as RFC.

Is there anything from your side I should take into account when doing that?

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

RE: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Radhey Shyam Pandey]

Hi,

> -Original Message-
> From: Peter Ujfalusi [mailto:peter.ujfal...@ti.com]
> Sent: Tuesday, April 24, 2018 3:21 PM
> To: Vinod Koul <vinod.k...@intel.com>
> Cc: Lars-Peter Clausen <l...@metafoo.de>; Radhey Shyam Pandey
> <radh...@xilinx.com>; michal.si...@xilinx.com; linux-
> ker...@vger.kernel.org; dmaeng...@vger.kernel.org;
> dan.j.willi...@intel.com; Appana Durga Kedareswara Rao
> <appa...@xilinx.com>; linux-arm-ker...@lists.infradead.org
> Subject: Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words
> to netdev dma client
> 
> On 2018-04-24 06:55, Vinod Koul wrote:
> > On Thu, Apr 19, 2018 at 02:40:26PM +0300, Peter Ujfalusi wrote:
> >>
> >> On 2018-04-18 16:06, Lars-Peter Clausen wrote:
> >>>> Hrm, true, but it is hardly the metadata use case. It is more like
> >>>> different DMA transfer type.
> >>>
> >>> When I look at this with my astronaut architect view from high high up
> above
> >>> I do not see a difference between metadata and multi-planar data.
> >>
> >> I tend to disagree.
> >
> > and we will love to hear more :)
> 
> It is getting pretty off topic from the subject ;) and I'm sorry about that.
> 
> Multi-planar data is _data_, the metadata is
> parameters/commands/information on _how_ to use the data.
> It is more like a replacement or extension of:
> configure peripheral
> send data
> 
> to
> 
> send data with configuration
> 
> In both cases the same data is sent, but the configuration,
> parametrization is 'simplified' to allow per packet changes.
> 
> >>> Both split the data that is sent to the peripheral into multiple
> >>> sub-streams, each carrying part of the data. I'm sure there are 
> >>> peripherals
> >>> that interleave data and metadata on the same data stream. Similar to
> how we
> >>> have left and right channel interleaved in a audio stream.
> >>
> >> Slimbus, S/PDIF?
> >>
> >>> What about metadata that is not contiguous and split into multiple
> segments.
> >>> How do you handle passing a sgl to the metadata interface? And then it
> >>> suddenly looks quite similar to the normal DMA descriptor interface.
> >>
> >> Well, the metadata is for the descriptor. The descriptor describe the
> >> data transfer _and_ can convey additional information. Nothing is
> >> interleaved, the data and the descriptor are different things. It is
> >> more like TCP headers detached from the data (but pointing to it).
> >>
> >>> But maybe that's just one abstraction level to high.
> >>
> >> I understand your point, but at the end the metadata needs to end up in
> >> the descriptor which is describing the data that is going to be moved.
> >>
> >> The descriptor is not sent as a separate DMA trasnfer, it is part of the
> >> DMA transfer, it is handled internally by the DMA.
> >
> > That is bit confusing to me. I thought DMA was transparent to meta data and
> > would blindly collect and transfer along with the descriptor. So at high
> > level we are talking about two transfers (probably co-joined at hip and you
> > want to call one transfer)
> 
> At the end yes, both the descriptor and the data is going to be sent to
> the other end.
> 
> As a reference see [1]
> 
> The metadata is not a separate entity, it is part of the descriptor
> (Host Packet Descriptor - HPD).
> Each transfer (packet) is described with a HPD. The HPD have optional
> fields, like EPIB (Extended Packet Info Block), PSdata (Protocol
> Specific data).
> 
> When the DMA reads the HPD, is going to move the data described by the
> HPD to the entry point (or from the entry point to memory), copies the
> EPIB/PSdata from the HPD to a destination HPD. The other end will use
> the destination HPD to know the size of the data and to get the metadata
> from the descriptor.
> 
> In essence every entity within the Multicore Navigator system have
> pktdma, they all work in a similar way, but their capabilities might
> differ. Our entry to this mesh is via the DMA.
> 
> > but why can't we visualize this as just a DMA
> > transfers. maybe you want to signal/attach to transfer, cant we do that with
> > additional flag DMA_METADATA etc..?
> 
> For the data we need to call dmaengine_prep_slave_* to create the
> descriptor (HPD). The metadata needs to be present in the HPD, hence I
> was thinking of the attach_metadata as per descriptor API.
> 
> If separate dmaengine_prep_slave_* is used for allocating the HPD a

RE: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Radhey Shyam Pandey]

Hi,

> -Original Message-
> From: Peter Ujfalusi [mailto:peter.ujfal...@ti.com]
> Sent: Tuesday, April 24, 2018 3:21 PM
> To: Vinod Koul 
> Cc: Lars-Peter Clausen ; Radhey Shyam Pandey
> ; michal.si...@xilinx.com; linux-
> ker...@vger.kernel.org; dmaeng...@vger.kernel.org;
> dan.j.willi...@intel.com; Appana Durga Kedareswara Rao
> ; linux-arm-ker...@lists.infradead.org
> Subject: Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words
> to netdev dma client
> 
> On 2018-04-24 06:55, Vinod Koul wrote:
> > On Thu, Apr 19, 2018 at 02:40:26PM +0300, Peter Ujfalusi wrote:
> >>
> >> On 2018-04-18 16:06, Lars-Peter Clausen wrote:
> >>>> Hrm, true, but it is hardly the metadata use case. It is more like
> >>>> different DMA transfer type.
> >>>
> >>> When I look at this with my astronaut architect view from high high up
> above
> >>> I do not see a difference between metadata and multi-planar data.
> >>
> >> I tend to disagree.
> >
> > and we will love to hear more :)
> 
> It is getting pretty off topic from the subject ;) and I'm sorry about that.
> 
> Multi-planar data is _data_, the metadata is
> parameters/commands/information on _how_ to use the data.
> It is more like a replacement or extension of:
> configure peripheral
> send data
> 
> to
> 
> send data with configuration
> 
> In both cases the same data is sent, but the configuration,
> parametrization is 'simplified' to allow per packet changes.
> 
> >>> Both split the data that is sent to the peripheral into multiple
> >>> sub-streams, each carrying part of the data. I'm sure there are 
> >>> peripherals
> >>> that interleave data and metadata on the same data stream. Similar to
> how we
> >>> have left and right channel interleaved in a audio stream.
> >>
> >> Slimbus, S/PDIF?
> >>
> >>> What about metadata that is not contiguous and split into multiple
> segments.
> >>> How do you handle passing a sgl to the metadata interface? And then it
> >>> suddenly looks quite similar to the normal DMA descriptor interface.
> >>
> >> Well, the metadata is for the descriptor. The descriptor describe the
> >> data transfer _and_ can convey additional information. Nothing is
> >> interleaved, the data and the descriptor are different things. It is
> >> more like TCP headers detached from the data (but pointing to it).
> >>
> >>> But maybe that's just one abstraction level to high.
> >>
> >> I understand your point, but at the end the metadata needs to end up in
> >> the descriptor which is describing the data that is going to be moved.
> >>
> >> The descriptor is not sent as a separate DMA trasnfer, it is part of the
> >> DMA transfer, it is handled internally by the DMA.
> >
> > That is bit confusing to me. I thought DMA was transparent to meta data and
> > would blindly collect and transfer along with the descriptor. So at high
> > level we are talking about two transfers (probably co-joined at hip and you
> > want to call one transfer)
> 
> At the end yes, both the descriptor and the data is going to be sent to
> the other end.
> 
> As a reference see [1]
> 
> The metadata is not a separate entity, it is part of the descriptor
> (Host Packet Descriptor - HPD).
> Each transfer (packet) is described with a HPD. The HPD have optional
> fields, like EPIB (Extended Packet Info Block), PSdata (Protocol
> Specific data).
> 
> When the DMA reads the HPD, is going to move the data described by the
> HPD to the entry point (or from the entry point to memory), copies the
> EPIB/PSdata from the HPD to a destination HPD. The other end will use
> the destination HPD to know the size of the data and to get the metadata
> from the descriptor.
> 
> In essence every entity within the Multicore Navigator system have
> pktdma, they all work in a similar way, but their capabilities might
> differ. Our entry to this mesh is via the DMA.
> 
> > but why can't we visualize this as just a DMA
> > transfers. maybe you want to signal/attach to transfer, cant we do that with
> > additional flag DMA_METADATA etc..?
> 
> For the data we need to call dmaengine_prep_slave_* to create the
> descriptor (HPD). The metadata needs to be present in the HPD, hence I
> was thinking of the attach_metadata as per descriptor API.
> 
> If separate dmaengine_prep_slave_* is used for allocating the HPD and
> place the metadata in it then the consequent dmaengine_prep_slave_* call
> must be for t

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Peter Ujfalusi]

On 2018-04-24 06:55, Vinod Koul wrote:
> On Thu, Apr 19, 2018 at 02:40:26PM +0300, Peter Ujfalusi wrote:
>>
>> On 2018-04-18 16:06, Lars-Peter Clausen wrote:
 Hrm, true, but it is hardly the metadata use case. It is more like
 different DMA transfer type.
>>>
>>> When I look at this with my astronaut architect view from high high up above
>>> I do not see a difference between metadata and multi-planar data.
>>
>> I tend to disagree.
> 
> and we will love to hear more :)

It is getting pretty off topic from the subject ;) and I'm sorry about that.

Multi-planar data is _data_, the metadata is
parameters/commands/information on _how_ to use the data.
It is more like a replacement or extension of:
configure peripheral
send data

to

send data with configuration

In both cases the same data is sent, but the configuration,
parametrization is 'simplified' to allow per packet changes.

>>> Both split the data that is sent to the peripheral into multiple
>>> sub-streams, each carrying part of the data. I'm sure there are peripherals
>>> that interleave data and metadata on the same data stream. Similar to how we
>>> have left and right channel interleaved in a audio stream.
>>
>> Slimbus, S/PDIF?
>>
>>> What about metadata that is not contiguous and split into multiple segments.
>>> How do you handle passing a sgl to the metadata interface? And then it
>>> suddenly looks quite similar to the normal DMA descriptor interface.
>>
>> Well, the metadata is for the descriptor. The descriptor describe the
>> data transfer _and_ can convey additional information. Nothing is
>> interleaved, the data and the descriptor are different things. It is
>> more like TCP headers detached from the data (but pointing to it).
>>
>>> But maybe that's just one abstraction level to high.
>>
>> I understand your point, but at the end the metadata needs to end up in
>> the descriptor which is describing the data that is going to be moved.
>>
>> The descriptor is not sent as a separate DMA trasnfer, it is part of the
>> DMA transfer, it is handled internally by the DMA.
> 
> That is bit confusing to me. I thought DMA was transparent to meta data and
> would blindly collect and transfer along with the descriptor. So at high
> level we are talking about two transfers (probably co-joined at hip and you
> want to call one transfer)

At the end yes, both the descriptor and the data is going to be sent to
the other end.

As a reference see [1]

The metadata is not a separate entity, it is part of the descriptor
(Host Packet Descriptor - HPD).
Each transfer (packet) is described with a HPD. The HPD have optional
fields, like EPIB (Extended Packet Info Block), PSdata (Protocol
Specific data).

When the DMA reads the HPD, is going to move the data described by the
HPD to the entry point (or from the entry point to memory), copies the
EPIB/PSdata from the HPD to a destination HPD. The other end will use
the destination HPD to know the size of the data and to get the metadata
from the descriptor.

In essence every entity within the Multicore Navigator system have
pktdma, they all work in a similar way, but their capabilities might
differ. Our entry to this mesh is via the DMA.

> but why can't we visualize this as just a DMA
> transfers. maybe you want to signal/attach to transfer, cant we do that with
> additional flag DMA_METADATA etc..?

For the data we need to call dmaengine_prep_slave_* to create the
descriptor (HPD). The metadata needs to be present in the HPD, hence I
was thinking of the attach_metadata as per descriptor API.

If separate dmaengine_prep_slave_* is used for allocating the HPD and
place the metadata in it then the consequent dmaengine_prep_slave_* call
must be for the data of the transfer and it is still unclear how the
prepare call would have any idea where to look for the HPD it needs to
update with the parameters for the data transfer.

I guess the driver could store the HPD pointer in the channel data if
the prepare is called with DMA_METADATA and it would be mandatory that
the next prepare is for the data portion. The driver would pick the
pointer to the HPD we stored away and update the descriptor belonging to
different tx_desc.

But if we are here, we could have a flag like DMA_DESCRIPTOR and let
client drivers to allocate the whole descriptor, fill in the metadata
and give that to the DMA driver, which will update the rest of the HPD.

Well, let's see where this is going to go when I can send the patches
for review.

[1] http://www.ti.com/lit/ug/sprugr9h/sprugr9h.pdf

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Peter Ujfalusi]

On 2018-04-24 06:55, Vinod Koul wrote:
> On Thu, Apr 19, 2018 at 02:40:26PM +0300, Peter Ujfalusi wrote:
>>
>> On 2018-04-18 16:06, Lars-Peter Clausen wrote:
 Hrm, true, but it is hardly the metadata use case. It is more like
 different DMA transfer type.
>>>
>>> When I look at this with my astronaut architect view from high high up above
>>> I do not see a difference between metadata and multi-planar data.
>>
>> I tend to disagree.
> 
> and we will love to hear more :)

It is getting pretty off topic from the subject ;) and I'm sorry about that.

Multi-planar data is _data_, the metadata is
parameters/commands/information on _how_ to use the data.
It is more like a replacement or extension of:
configure peripheral
send data

to

send data with configuration

In both cases the same data is sent, but the configuration,
parametrization is 'simplified' to allow per packet changes.

>>> Both split the data that is sent to the peripheral into multiple
>>> sub-streams, each carrying part of the data. I'm sure there are peripherals
>>> that interleave data and metadata on the same data stream. Similar to how we
>>> have left and right channel interleaved in a audio stream.
>>
>> Slimbus, S/PDIF?
>>
>>> What about metadata that is not contiguous and split into multiple segments.
>>> How do you handle passing a sgl to the metadata interface? And then it
>>> suddenly looks quite similar to the normal DMA descriptor interface.
>>
>> Well, the metadata is for the descriptor. The descriptor describe the
>> data transfer _and_ can convey additional information. Nothing is
>> interleaved, the data and the descriptor are different things. It is
>> more like TCP headers detached from the data (but pointing to it).
>>
>>> But maybe that's just one abstraction level to high.
>>
>> I understand your point, but at the end the metadata needs to end up in
>> the descriptor which is describing the data that is going to be moved.
>>
>> The descriptor is not sent as a separate DMA trasnfer, it is part of the
>> DMA transfer, it is handled internally by the DMA.
> 
> That is bit confusing to me. I thought DMA was transparent to meta data and
> would blindly collect and transfer along with the descriptor. So at high
> level we are talking about two transfers (probably co-joined at hip and you
> want to call one transfer)

At the end yes, both the descriptor and the data is going to be sent to
the other end.

As a reference see [1]

The metadata is not a separate entity, it is part of the descriptor
(Host Packet Descriptor - HPD).
Each transfer (packet) is described with a HPD. The HPD have optional
fields, like EPIB (Extended Packet Info Block), PSdata (Protocol
Specific data).

When the DMA reads the HPD, is going to move the data described by the
HPD to the entry point (or from the entry point to memory), copies the
EPIB/PSdata from the HPD to a destination HPD. The other end will use
the destination HPD to know the size of the data and to get the metadata
from the descriptor.

In essence every entity within the Multicore Navigator system have
pktdma, they all work in a similar way, but their capabilities might
differ. Our entry to this mesh is via the DMA.

> but why can't we visualize this as just a DMA
> transfers. maybe you want to signal/attach to transfer, cant we do that with
> additional flag DMA_METADATA etc..?

For the data we need to call dmaengine_prep_slave_* to create the
descriptor (HPD). The metadata needs to be present in the HPD, hence I
was thinking of the attach_metadata as per descriptor API.

If separate dmaengine_prep_slave_* is used for allocating the HPD and
place the metadata in it then the consequent dmaengine_prep_slave_* call
must be for the data of the transfer and it is still unclear how the
prepare call would have any idea where to look for the HPD it needs to
update with the parameters for the data transfer.

I guess the driver could store the HPD pointer in the channel data if
the prepare is called with DMA_METADATA and it would be mandatory that
the next prepare is for the data portion. The driver would pick the
pointer to the HPD we stored away and update the descriptor belonging to
different tx_desc.

But if we are here, we could have a flag like DMA_DESCRIPTOR and let
client drivers to allocate the whole descriptor, fill in the metadata
and give that to the DMA driver, which will update the rest of the HPD.

Well, let's see where this is going to go when I can send the patches
for review.

[1] http://www.ti.com/lit/ug/sprugr9h/sprugr9h.pdf

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Vinod Koul]

On Thu, Apr 19, 2018 at 02:40:26PM +0300, Peter Ujfalusi wrote:
> 
> On 2018-04-18 16:06, Lars-Peter Clausen wrote:
> >> Hrm, true, but it is hardly the metadata use case. It is more like
> >> different DMA transfer type.
> > 
> > When I look at this with my astronaut architect view from high high up above
> > I do not see a difference between metadata and multi-planar data.
> 
> I tend to disagree.

and we will love to hear more :)

> > Both split the data that is sent to the peripheral into multiple
> > sub-streams, each carrying part of the data. I'm sure there are peripherals
> > that interleave data and metadata on the same data stream. Similar to how we
> > have left and right channel interleaved in a audio stream.
> 
> Slimbus, S/PDIF?
> 
> > What about metadata that is not contiguous and split into multiple segments.
> > How do you handle passing a sgl to the metadata interface? And then it
> > suddenly looks quite similar to the normal DMA descriptor interface.
> 
> Well, the metadata is for the descriptor. The descriptor describe the
> data transfer _and_ can convey additional information. Nothing is
> interleaved, the data and the descriptor are different things. It is
> more like TCP headers detached from the data (but pointing to it).
> 
> > But maybe that's just one abstraction level to high.
> 
> I understand your point, but at the end the metadata needs to end up in
> the descriptor which is describing the data that is going to be moved.
> 
> The descriptor is not sent as a separate DMA trasnfer, it is part of the
> DMA transfer, it is handled internally by the DMA.

That is bit confusing to me. I thought DMA was transparent to meta data and
would blindly collect and transfer along with the descriptor. So at high
level we are talking about two transfers (probably co-joined at hip and you
want to call one transfer) but why can't we visualize this as just a DMA
transfers. maybe you want to signal/attach to transfer, cant we do that with
additional flag DMA_METADATA etc..?

-- 
~Vinod

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Vinod Koul]

On Thu, Apr 19, 2018 at 02:40:26PM +0300, Peter Ujfalusi wrote:
> 
> On 2018-04-18 16:06, Lars-Peter Clausen wrote:
> >> Hrm, true, but it is hardly the metadata use case. It is more like
> >> different DMA transfer type.
> > 
> > When I look at this with my astronaut architect view from high high up above
> > I do not see a difference between metadata and multi-planar data.
> 
> I tend to disagree.

and we will love to hear more :)

> > Both split the data that is sent to the peripheral into multiple
> > sub-streams, each carrying part of the data. I'm sure there are peripherals
> > that interleave data and metadata on the same data stream. Similar to how we
> > have left and right channel interleaved in a audio stream.
> 
> Slimbus, S/PDIF?
> 
> > What about metadata that is not contiguous and split into multiple segments.
> > How do you handle passing a sgl to the metadata interface? And then it
> > suddenly looks quite similar to the normal DMA descriptor interface.
> 
> Well, the metadata is for the descriptor. The descriptor describe the
> data transfer _and_ can convey additional information. Nothing is
> interleaved, the data and the descriptor are different things. It is
> more like TCP headers detached from the data (but pointing to it).
> 
> > But maybe that's just one abstraction level to high.
> 
> I understand your point, but at the end the metadata needs to end up in
> the descriptor which is describing the data that is going to be moved.
> 
> The descriptor is not sent as a separate DMA trasnfer, it is part of the
> DMA transfer, it is handled internally by the DMA.

That is bit confusing to me. I thought DMA was transparent to meta data and
would blindly collect and transfer along with the descriptor. So at high
level we are talking about two transfers (probably co-joined at hip and you
want to call one transfer) but why can't we visualize this as just a DMA
transfers. maybe you want to signal/attach to transfer, cant we do that with
additional flag DMA_METADATA etc..?

-- 
~Vinod

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Peter Ujfalusi]

On 2018-04-18 16:06, Lars-Peter Clausen wrote:
>> Hrm, true, but it is hardly the metadata use case. It is more like
>> different DMA transfer type.
> 
> When I look at this with my astronaut architect view from high high up above
> I do not see a difference between metadata and multi-planar data.

I tend to disagree.

> Both split the data that is sent to the peripheral into multiple
> sub-streams, each carrying part of the data. I'm sure there are peripherals
> that interleave data and metadata on the same data stream. Similar to how we
> have left and right channel interleaved in a audio stream.

Slimbus, S/PDIF?

> What about metadata that is not contiguous and split into multiple segments.
> How do you handle passing a sgl to the metadata interface? And then it
> suddenly looks quite similar to the normal DMA descriptor interface.

Well, the metadata is for the descriptor. The descriptor describe the
data transfer _and_ can convey additional information. Nothing is
interleaved, the data and the descriptor are different things. It is
more like TCP headers detached from the data (but pointing to it).

> But maybe that's just one abstraction level to high.

I understand your point, but at the end the metadata needs to end up in
the descriptor which is describing the data that is going to be moved.

The descriptor is not sent as a separate DMA trasnfer, it is part of the
DMA transfer, it is handled internally by the DMA.

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Peter Ujfalusi]

On 2018-04-18 16:06, Lars-Peter Clausen wrote:
>> Hrm, true, but it is hardly the metadata use case. It is more like
>> different DMA transfer type.
> 
> When I look at this with my astronaut architect view from high high up above
> I do not see a difference between metadata and multi-planar data.

I tend to disagree.

> Both split the data that is sent to the peripheral into multiple
> sub-streams, each carrying part of the data. I'm sure there are peripherals
> that interleave data and metadata on the same data stream. Similar to how we
> have left and right channel interleaved in a audio stream.

Slimbus, S/PDIF?

> What about metadata that is not contiguous and split into multiple segments.
> How do you handle passing a sgl to the metadata interface? And then it
> suddenly looks quite similar to the normal DMA descriptor interface.

Well, the metadata is for the descriptor. The descriptor describe the
data transfer _and_ can convey additional information. Nothing is
interleaved, the data and the descriptor are different things. It is
more like TCP headers detached from the data (but pointing to it).

> But maybe that's just one abstraction level to high.

I understand your point, but at the end the metadata needs to end up in
the descriptor which is describing the data that is going to be moved.

The descriptor is not sent as a separate DMA trasnfer, it is part of the
DMA transfer, it is handled internally by the DMA.

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Lars-Peter Clausen]

On 04/18/2018 08:31 AM, Peter Ujfalusi wrote:
> 
> On 2018-04-17 18:54, Lars-Peter Clausen wrote:
>> On 04/17/2018 04:53 PM, Peter Ujfalusi wrote:
>>> On 2018-04-17 16:58, Lars-Peter Clausen wrote:
>> There are two options.
>>
>> Either you extend the generic interfaces so it can cover your usecase in 
>> a
>> generic way. E.g. the ability to attach meta data to transfer.
>
> Fwiw I have this patch as part of a bigger work to achieve similar 
> results:

 That's good stuff. Is this in a public tree somewhere?
>>>
>>> Not atm. I can not send the user of the new API and I did not wanted to
>>> send something like this out of the blue w/o context.
>>>
>>> But as it is a generic patch, I can send it as well. The only thing is
>>> that the need for the memcpy, so I might end up with
>>> ptr = get_metadata_ptr(desc, ); /* size: in RX the valid size */
>>>
>>> and set_metadata_size(); /* in TX to tell how the client placed */
>>>
>>> Or something like that, the attach_metadata() as it is works just fine,
>>> but high throughput might not like the memcpy.
>>>
>>
>> In the most abstracted way I'd say metadata and data are two different data
>> streams that are correlated and send/received at the same time.
> 
> In my case the meatdata is sideband information or parameters for/from
> the remote end. Like timestamp, algorithm parameters, keys, etc.
> 
> It is tight to the data payload, but it is not part of it.
> 
> But the API should be generic enough to cover other use cases where
> clients need to provide additional information.
> For me, the metadata is part of the descriptor we give and receive back
> from the DMA, others might have sideband channel to send that.
> 
> For metadata handling we could have:
> 
> struct dma_desc_metadata_ops {
>/* To give a buffer for the DMA with the metadata, as it was in my
> * original patch
> */
>int (*desc_attach_metadata)(struct dma_async_tx_descriptor *desc,
>void *data, size_t len);
> 
>void *(*desc_get_metadata_ptr)(struct dma_async_tx_descriptor *desc,
>   size_t *payload_len, size_t *max_len);
>int (*desc_set_payload_len)(struct dma_async_tx_descriptor *desc,
>   size_t payload_len);
> };
> 
> Probably a simple flag variable to indicate which of the two modes are
> supported:
> 1. Client provided metadata buffer handling
> Clients provide the buffer via desc_attach_metadata(), the DMA driver
> will do whatever it needs to do, copy it in place, send it differently,
> use parameters.
> In RX the received metadata is going to be placed to the provided buffer.
> 2. Ability to give the metadata pointer to user to work on it.
> In TX, clients can use desc_get_metadata_ptr() to get the pointer,
> current payload size and maximum size of the metadata and can work
> directly on the buffer to place the data. Then desc_set_payload_len() to
> let the DMA know how much data is actually placed there.
> In RX, desc_get_metadata_ptr() will give the user the pointer and the
> payload size so it can process that information correctly.
> 
> DMA driver can implement either or both, but clients must only use
> either 1 or 2 to work with the metadata.
> 
> 
>> Think multi-planar transfer, like for audio when the right and left channel
>> are in separate buffers and not interleaved. Or video with different
>> color/luminance components in separate buffers. This is something that is at
>> the moment not covered by the dmaengine API either.
> 
> Hrm, true, but it is hardly the metadata use case. It is more like
> different DMA transfer type.

When I look at this with my astronaut architect view from high high up above
I do not see a difference between metadata and multi-planar data.

Both split the data that is sent to the peripheral into multiple
sub-streams, each carrying part of the data. I'm sure there are peripherals
that interleave data and metadata on the same data stream. Similar to how we
have left and right channel interleaved in a audio stream.

What about metadata that is not contiguous and split into multiple segments.
How do you handle passing a sgl to the metadata interface? And then it
suddenly looks quite similar to the normal DMA descriptor interface.

But maybe that's just one abstraction level to high.

>> Or you can implement a interface that is specific to your DMA controller 
>> and
>> any client using this interface knows it is talking to your DMA 
>> controller.
>
> Hrm, so we can have DMA driver specific calls? The reason why TI's 
> keystone 2
> navigator DMA support was rejected that it was introducing NAV specific 
> calls
> for clients to configure features not yet supported by the framework.

 In my opinion it is OK, somebody else might have different ideas. I mean it
 is not nice, but it is better than the alternative of overloading the
 generic API with 

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Lars-Peter Clausen]

On 04/18/2018 08:31 AM, Peter Ujfalusi wrote:
> 
> On 2018-04-17 18:54, Lars-Peter Clausen wrote:
>> On 04/17/2018 04:53 PM, Peter Ujfalusi wrote:
>>> On 2018-04-17 16:58, Lars-Peter Clausen wrote:
>> There are two options.
>>
>> Either you extend the generic interfaces so it can cover your usecase in 
>> a
>> generic way. E.g. the ability to attach meta data to transfer.
>
> Fwiw I have this patch as part of a bigger work to achieve similar 
> results:

 That's good stuff. Is this in a public tree somewhere?
>>>
>>> Not atm. I can not send the user of the new API and I did not wanted to
>>> send something like this out of the blue w/o context.
>>>
>>> But as it is a generic patch, I can send it as well. The only thing is
>>> that the need for the memcpy, so I might end up with
>>> ptr = get_metadata_ptr(desc, ); /* size: in RX the valid size */
>>>
>>> and set_metadata_size(); /* in TX to tell how the client placed */
>>>
>>> Or something like that, the attach_metadata() as it is works just fine,
>>> but high throughput might not like the memcpy.
>>>
>>
>> In the most abstracted way I'd say metadata and data are two different data
>> streams that are correlated and send/received at the same time.
> 
> In my case the meatdata is sideband information or parameters for/from
> the remote end. Like timestamp, algorithm parameters, keys, etc.
> 
> It is tight to the data payload, but it is not part of it.
> 
> But the API should be generic enough to cover other use cases where
> clients need to provide additional information.
> For me, the metadata is part of the descriptor we give and receive back
> from the DMA, others might have sideband channel to send that.
> 
> For metadata handling we could have:
> 
> struct dma_desc_metadata_ops {
>/* To give a buffer for the DMA with the metadata, as it was in my
> * original patch
> */
>int (*desc_attach_metadata)(struct dma_async_tx_descriptor *desc,
>void *data, size_t len);
> 
>void *(*desc_get_metadata_ptr)(struct dma_async_tx_descriptor *desc,
>   size_t *payload_len, size_t *max_len);
>int (*desc_set_payload_len)(struct dma_async_tx_descriptor *desc,
>   size_t payload_len);
> };
> 
> Probably a simple flag variable to indicate which of the two modes are
> supported:
> 1. Client provided metadata buffer handling
> Clients provide the buffer via desc_attach_metadata(), the DMA driver
> will do whatever it needs to do, copy it in place, send it differently,
> use parameters.
> In RX the received metadata is going to be placed to the provided buffer.
> 2. Ability to give the metadata pointer to user to work on it.
> In TX, clients can use desc_get_metadata_ptr() to get the pointer,
> current payload size and maximum size of the metadata and can work
> directly on the buffer to place the data. Then desc_set_payload_len() to
> let the DMA know how much data is actually placed there.
> In RX, desc_get_metadata_ptr() will give the user the pointer and the
> payload size so it can process that information correctly.
> 
> DMA driver can implement either or both, but clients must only use
> either 1 or 2 to work with the metadata.
> 
> 
>> Think multi-planar transfer, like for audio when the right and left channel
>> are in separate buffers and not interleaved. Or video with different
>> color/luminance components in separate buffers. This is something that is at
>> the moment not covered by the dmaengine API either.
> 
> Hrm, true, but it is hardly the metadata use case. It is more like
> different DMA transfer type.

When I look at this with my astronaut architect view from high high up above
I do not see a difference between metadata and multi-planar data.

Both split the data that is sent to the peripheral into multiple
sub-streams, each carrying part of the data. I'm sure there are peripherals
that interleave data and metadata on the same data stream. Similar to how we
have left and right channel interleaved in a audio stream.

What about metadata that is not contiguous and split into multiple segments.
How do you handle passing a sgl to the metadata interface? And then it
suddenly looks quite similar to the normal DMA descriptor interface.

But maybe that's just one abstraction level to high.

>> Or you can implement a interface that is specific to your DMA controller 
>> and
>> any client using this interface knows it is talking to your DMA 
>> controller.
>
> Hrm, so we can have DMA driver specific calls? The reason why TI's 
> keystone 2
> navigator DMA support was rejected that it was introducing NAV specific 
> calls
> for clients to configure features not yet supported by the framework.

 In my opinion it is OK, somebody else might have different ideas. I mean it
 is not nice, but it is better than the alternative of overloading the
 generic API with 

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Peter Ujfalusi]

On 2018-04-18 09:39, Peter Ujfalusi wrote:
> 
> 
> On 2018-04-17 18:42, Vinod Koul wrote:
>> On Tue, Apr 17, 2018 at 04:46:43PM +0300, Peter Ujfalusi wrote:
>>
>>> @@ -709,6 +709,11 @@ struct dma_filter {
>>>   * be called after period_len bytes have been transferred.
>>>   * @device_prep_interleaved_dma: Transfer expression in a generic way.
>>>   * @device_prep_dma_imm_data: DMA's 8 byte immediate data to the dst 
>>> address
>>> + * @device_attach_metadata: Some DMA engines can send and receive side band
>>> + * information, commands or parameters which is not transferred within the
>>> + * data stream itself. In such case clients can set the metadata to the
>>> + * given descriptor and it is going to be sent to the peripheral, or in
>>> + * case of DEV_TO_MEM the provided buffer will receive the metadata.
>>>   * @device_config: Pushes a new configuration to a channel, return 0 or an 
>>> error
>>>   * code
>>>   * @device_pause: Pauses any transfer happening on a channel. Returns
>>> @@ -796,6 +801,9 @@ struct dma_device {
>>> struct dma_chan *chan, dma_addr_t dst, u64 data,
>>> unsigned long flags);
>>>  
>>> +   int (*device_attach_metadata)(struct dma_async_tx_descriptor *desc,
>>> + void *data, size_t len);
>>
>> while i am okay with the concept, I would not want to go again the custom
>> pointer route, this is a no-go for me.
>>
>> Instead lets add the vendor data, define that explicitly. We can use struct,
>> tokens or something else to define these. But lets try to stay away from
>> opaque objects please :-)
> 
> The DMA does not interpret the metadata, it is information which can be
> only understood by the client driver and the remote peripheral. It is
> just chunk of data (parameters, timestamps, keys, etc) that needs to
> travel along with the payload.
> 
> The content is not relevant for the DMA itself.

To add: different peripherals needs to send receive different metadata
and even the same peripheral might pass different information based on
their operating mode. The size of metadata can be different as well.

So it is not really vendor specific metadata, but peripheral, operating
mode and other factors affected chunk of data.

> 
> - Péter
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Peter Ujfalusi]

On 2018-04-18 09:39, Peter Ujfalusi wrote:
> 
> 
> On 2018-04-17 18:42, Vinod Koul wrote:
>> On Tue, Apr 17, 2018 at 04:46:43PM +0300, Peter Ujfalusi wrote:
>>
>>> @@ -709,6 +709,11 @@ struct dma_filter {
>>>   * be called after period_len bytes have been transferred.
>>>   * @device_prep_interleaved_dma: Transfer expression in a generic way.
>>>   * @device_prep_dma_imm_data: DMA's 8 byte immediate data to the dst 
>>> address
>>> + * @device_attach_metadata: Some DMA engines can send and receive side band
>>> + * information, commands or parameters which is not transferred within the
>>> + * data stream itself. In such case clients can set the metadata to the
>>> + * given descriptor and it is going to be sent to the peripheral, or in
>>> + * case of DEV_TO_MEM the provided buffer will receive the metadata.
>>>   * @device_config: Pushes a new configuration to a channel, return 0 or an 
>>> error
>>>   * code
>>>   * @device_pause: Pauses any transfer happening on a channel. Returns
>>> @@ -796,6 +801,9 @@ struct dma_device {
>>> struct dma_chan *chan, dma_addr_t dst, u64 data,
>>> unsigned long flags);
>>>  
>>> +   int (*device_attach_metadata)(struct dma_async_tx_descriptor *desc,
>>> + void *data, size_t len);
>>
>> while i am okay with the concept, I would not want to go again the custom
>> pointer route, this is a no-go for me.
>>
>> Instead lets add the vendor data, define that explicitly. We can use struct,
>> tokens or something else to define these. But lets try to stay away from
>> opaque objects please :-)
> 
> The DMA does not interpret the metadata, it is information which can be
> only understood by the client driver and the remote peripheral. It is
> just chunk of data (parameters, timestamps, keys, etc) that needs to
> travel along with the payload.
> 
> The content is not relevant for the DMA itself.

To add: different peripherals needs to send receive different metadata
and even the same peripheral might pass different information based on
their operating mode. The size of metadata can be different as well.

So it is not really vendor specific metadata, but peripheral, operating
mode and other factors affected chunk of data.

> 
> - Péter
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Peter Ujfalusi]

On 2018-04-17 18:42, Vinod Koul wrote:
> On Tue, Apr 17, 2018 at 04:46:43PM +0300, Peter Ujfalusi wrote:
> 
>> @@ -709,6 +709,11 @@ struct dma_filter {
>>   *  be called after period_len bytes have been transferred.
>>   * @device_prep_interleaved_dma: Transfer expression in a generic way.
>>   * @device_prep_dma_imm_data: DMA's 8 byte immediate data to the dst address
>> + * @device_attach_metadata: Some DMA engines can send and receive side band
>> + *  information, commands or parameters which is not transferred within the
>> + *  data stream itself. In such case clients can set the metadata to the
>> + *  given descriptor and it is going to be sent to the peripheral, or in
>> + *  case of DEV_TO_MEM the provided buffer will receive the metadata.
>>   * @device_config: Pushes a new configuration to a channel, return 0 or an 
>> error
>>   *  code
>>   * @device_pause: Pauses any transfer happening on a channel. Returns
>> @@ -796,6 +801,9 @@ struct dma_device {
>>  struct dma_chan *chan, dma_addr_t dst, u64 data,
>>  unsigned long flags);
>>  
>> +int (*device_attach_metadata)(struct dma_async_tx_descriptor *desc,
>> +  void *data, size_t len);
> 
> while i am okay with the concept, I would not want to go again the custom
> pointer route, this is a no-go for me.
> 
> Instead lets add the vendor data, define that explicitly. We can use struct,
> tokens or something else to define these. But lets try to stay away from
> opaque objects please :-)

The DMA does not interpret the metadata, it is information which can be
only understood by the client driver and the remote peripheral. It is
just chunk of data (parameters, timestamps, keys, etc) that needs to
travel along with the payload.

The content is not relevant for the DMA itself.

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Peter Ujfalusi]

On 2018-04-17 18:42, Vinod Koul wrote:
> On Tue, Apr 17, 2018 at 04:46:43PM +0300, Peter Ujfalusi wrote:
> 
>> @@ -709,6 +709,11 @@ struct dma_filter {
>>   *  be called after period_len bytes have been transferred.
>>   * @device_prep_interleaved_dma: Transfer expression in a generic way.
>>   * @device_prep_dma_imm_data: DMA's 8 byte immediate data to the dst address
>> + * @device_attach_metadata: Some DMA engines can send and receive side band
>> + *  information, commands or parameters which is not transferred within the
>> + *  data stream itself. In such case clients can set the metadata to the
>> + *  given descriptor and it is going to be sent to the peripheral, or in
>> + *  case of DEV_TO_MEM the provided buffer will receive the metadata.
>>   * @device_config: Pushes a new configuration to a channel, return 0 or an 
>> error
>>   *  code
>>   * @device_pause: Pauses any transfer happening on a channel. Returns
>> @@ -796,6 +801,9 @@ struct dma_device {
>>  struct dma_chan *chan, dma_addr_t dst, u64 data,
>>  unsigned long flags);
>>  
>> +int (*device_attach_metadata)(struct dma_async_tx_descriptor *desc,
>> +  void *data, size_t len);
> 
> while i am okay with the concept, I would not want to go again the custom
> pointer route, this is a no-go for me.
> 
> Instead lets add the vendor data, define that explicitly. We can use struct,
> tokens or something else to define these. But lets try to stay away from
> opaque objects please :-)

The DMA does not interpret the metadata, it is information which can be
only understood by the client driver and the remote peripheral. It is
just chunk of data (parameters, timestamps, keys, etc) that needs to
travel along with the payload.

The content is not relevant for the DMA itself.

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Peter Ujfalusi]

On 2018-04-17 18:54, Lars-Peter Clausen wrote:
> On 04/17/2018 04:53 PM, Peter Ujfalusi wrote:
>> On 2018-04-17 16:58, Lars-Peter Clausen wrote:
> There are two options.
>
> Either you extend the generic interfaces so it can cover your usecase in a
> generic way. E.g. the ability to attach meta data to transfer.

 Fwiw I have this patch as part of a bigger work to achieve similar results:
>>>
>>> That's good stuff. Is this in a public tree somewhere?
>>
>> Not atm. I can not send the user of the new API and I did not wanted to
>> send something like this out of the blue w/o context.
>>
>> But as it is a generic patch, I can send it as well. The only thing is
>> that the need for the memcpy, so I might end up with
>> ptr = get_metadata_ptr(desc, ); /* size: in RX the valid size */
>>
>> and set_metadata_size(); /* in TX to tell how the client placed */
>>
>> Or something like that, the attach_metadata() as it is works just fine,
>> but high throughput might not like the memcpy.
>>
> 
> In the most abstracted way I'd say metadata and data are two different data
> streams that are correlated and send/received at the same time.

In my case the meatdata is sideband information or parameters for/from
the remote end. Like timestamp, algorithm parameters, keys, etc.

It is tight to the data payload, but it is not part of it.

But the API should be generic enough to cover other use cases where
clients need to provide additional information.
For me, the metadata is part of the descriptor we give and receive back
from the DMA, others might have sideband channel to send that.

For metadata handling we could have:

struct dma_desc_metadata_ops {
   /* To give a buffer for the DMA with the metadata, as it was in my
* original patch
*/
   int (*desc_attach_metadata)(struct dma_async_tx_descriptor *desc,
   void *data, size_t len);

   void *(*desc_get_metadata_ptr)(struct dma_async_tx_descriptor *desc,
  size_t *payload_len, size_t *max_len);
   int (*desc_set_payload_len)(struct dma_async_tx_descriptor *desc,
  size_t payload_len);
};

Probably a simple flag variable to indicate which of the two modes are
supported:
1. Client provided metadata buffer handling
Clients provide the buffer via desc_attach_metadata(), the DMA driver
will do whatever it needs to do, copy it in place, send it differently,
use parameters.
In RX the received metadata is going to be placed to the provided buffer.
2. Ability to give the metadata pointer to user to work on it.
In TX, clients can use desc_get_metadata_ptr() to get the pointer,
current payload size and maximum size of the metadata and can work
directly on the buffer to place the data. Then desc_set_payload_len() to
let the DMA know how much data is actually placed there.
In RX, desc_get_metadata_ptr() will give the user the pointer and the
payload size so it can process that information correctly.

DMA driver can implement either or both, but clients must only use
either 1 or 2 to work with the metadata.


> Think multi-planar transfer, like for audio when the right and left channel
> are in separate buffers and not interleaved. Or video with different
> color/luminance components in separate buffers. This is something that is at
> the moment not covered by the dmaengine API either.

Hrm, true, but it is hardly the metadata use case. It is more like
different DMA transfer type.

> Or you can implement a interface that is specific to your DMA controller 
> and
> any client using this interface knows it is talking to your DMA 
> controller.

 Hrm, so we can have DMA driver specific calls? The reason why TI's 
 keystone 2
 navigator DMA support was rejected that it was introducing NAV specific 
 calls
 for clients to configure features not yet supported by the framework.
>>>
>>> In my opinion it is OK, somebody else might have different ideas. I mean it
>>> is not nice, but it is better than the alternative of overloading the
>>> generic API with driver specific semantics or introducing some kind of IOCTL
>>> catch all callback.
>>
>> True, but the generic API can be extended as well to cover new grounds,
>> features. Like this metadata thing.
>>
>>> If there is tight coupling between the DMA core and client and there is no
>>> intention of using a generic client the best solution might even be to no
>>> use DMAengine at all.
>>
>> This is how the knav stuff ended up. Well it is only used by networking
>> atm, so it is 'fine' to have custom API, but it is not portable.
> 
> I totally agree generic APIs are better, but not everybody has the resources
> to rewrite the whole framework just because they want to do this tiny thing
> that isn't covered by the framework yet. In that case it is better to go
> with a custom API (that might evolve into a generic API), rather than
> overloading the generic API and putting a strain on 

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Peter Ujfalusi]

On 2018-04-17 18:54, Lars-Peter Clausen wrote:
> On 04/17/2018 04:53 PM, Peter Ujfalusi wrote:
>> On 2018-04-17 16:58, Lars-Peter Clausen wrote:
> There are two options.
>
> Either you extend the generic interfaces so it can cover your usecase in a
> generic way. E.g. the ability to attach meta data to transfer.

 Fwiw I have this patch as part of a bigger work to achieve similar results:
>>>
>>> That's good stuff. Is this in a public tree somewhere?
>>
>> Not atm. I can not send the user of the new API and I did not wanted to
>> send something like this out of the blue w/o context.
>>
>> But as it is a generic patch, I can send it as well. The only thing is
>> that the need for the memcpy, so I might end up with
>> ptr = get_metadata_ptr(desc, ); /* size: in RX the valid size */
>>
>> and set_metadata_size(); /* in TX to tell how the client placed */
>>
>> Or something like that, the attach_metadata() as it is works just fine,
>> but high throughput might not like the memcpy.
>>
> 
> In the most abstracted way I'd say metadata and data are two different data
> streams that are correlated and send/received at the same time.

In my case the meatdata is sideband information or parameters for/from
the remote end. Like timestamp, algorithm parameters, keys, etc.

It is tight to the data payload, but it is not part of it.

But the API should be generic enough to cover other use cases where
clients need to provide additional information.
For me, the metadata is part of the descriptor we give and receive back
from the DMA, others might have sideband channel to send that.

For metadata handling we could have:

struct dma_desc_metadata_ops {
   /* To give a buffer for the DMA with the metadata, as it was in my
* original patch
*/
   int (*desc_attach_metadata)(struct dma_async_tx_descriptor *desc,
   void *data, size_t len);

   void *(*desc_get_metadata_ptr)(struct dma_async_tx_descriptor *desc,
  size_t *payload_len, size_t *max_len);
   int (*desc_set_payload_len)(struct dma_async_tx_descriptor *desc,
  size_t payload_len);
};

Probably a simple flag variable to indicate which of the two modes are
supported:
1. Client provided metadata buffer handling
Clients provide the buffer via desc_attach_metadata(), the DMA driver
will do whatever it needs to do, copy it in place, send it differently,
use parameters.
In RX the received metadata is going to be placed to the provided buffer.
2. Ability to give the metadata pointer to user to work on it.
In TX, clients can use desc_get_metadata_ptr() to get the pointer,
current payload size and maximum size of the metadata and can work
directly on the buffer to place the data. Then desc_set_payload_len() to
let the DMA know how much data is actually placed there.
In RX, desc_get_metadata_ptr() will give the user the pointer and the
payload size so it can process that information correctly.

DMA driver can implement either or both, but clients must only use
either 1 or 2 to work with the metadata.


> Think multi-planar transfer, like for audio when the right and left channel
> are in separate buffers and not interleaved. Or video with different
> color/luminance components in separate buffers. This is something that is at
> the moment not covered by the dmaengine API either.

Hrm, true, but it is hardly the metadata use case. It is more like
different DMA transfer type.

> Or you can implement a interface that is specific to your DMA controller 
> and
> any client using this interface knows it is talking to your DMA 
> controller.

 Hrm, so we can have DMA driver specific calls? The reason why TI's 
 keystone 2
 navigator DMA support was rejected that it was introducing NAV specific 
 calls
 for clients to configure features not yet supported by the framework.
>>>
>>> In my opinion it is OK, somebody else might have different ideas. I mean it
>>> is not nice, but it is better than the alternative of overloading the
>>> generic API with driver specific semantics or introducing some kind of IOCTL
>>> catch all callback.
>>
>> True, but the generic API can be extended as well to cover new grounds,
>> features. Like this metadata thing.
>>
>>> If there is tight coupling between the DMA core and client and there is no
>>> intention of using a generic client the best solution might even be to no
>>> use DMAengine at all.
>>
>> This is how the knav stuff ended up. Well it is only used by networking
>> atm, so it is 'fine' to have custom API, but it is not portable.
> 
> I totally agree generic APIs are better, but not everybody has the resources
> to rewrite the whole framework just because they want to do this tiny thing
> that isn't covered by the framework yet. In that case it is better to go
> with a custom API (that might evolve into a generic API), rather than
> overloading the generic API and putting a strain on 

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Lars-Peter Clausen]

On 04/17/2018 04:53 PM, Peter Ujfalusi wrote:
> On 2018-04-17 16:58, Lars-Peter Clausen wrote:
 There are two options.

 Either you extend the generic interfaces so it can cover your usecase in a
 generic way. E.g. the ability to attach meta data to transfer.
>>>
>>> Fwiw I have this patch as part of a bigger work to achieve similar results:
>>
>> That's good stuff. Is this in a public tree somewhere?
> 
> Not atm. I can not send the user of the new API and I did not wanted to
> send something like this out of the blue w/o context.
> 
> But as it is a generic patch, I can send it as well. The only thing is
> that the need for the memcpy, so I might end up with
> ptr = get_metadata_ptr(desc, ); /* size: in RX the valid size */
> 
> and set_metadata_size(); /* in TX to tell how the client placed */
> 
> Or something like that, the attach_metadata() as it is works just fine,
> but high throughput might not like the memcpy.
> 

In the most abstracted way I'd say metadata and data are two different data
streams that are correlated and send/received at the same time.

Think multi-planar transfer, like for audio when the right and left channel
are in separate buffers and not interleaved. Or video with different
color/luminance components in separate buffers. This is something that is at
the moment not covered by the dmaengine API either.

 Or you can implement a interface that is specific to your DMA controller 
 and
 any client using this interface knows it is talking to your DMA controller.
>>>
>>> Hrm, so we can have DMA driver specific calls? The reason why TI's keystone 
>>> 2
>>> navigator DMA support was rejected that it was introducing NAV specific 
>>> calls
>>> for clients to configure features not yet supported by the framework.
>>
>> In my opinion it is OK, somebody else might have different ideas. I mean it
>> is not nice, but it is better than the alternative of overloading the
>> generic API with driver specific semantics or introducing some kind of IOCTL
>> catch all callback.
> 
> True, but the generic API can be extended as well to cover new grounds,
> features. Like this metadata thing.
> 
>> If there is tight coupling between the DMA core and client and there is no
>> intention of using a generic client the best solution might even be to no
>> use DMAengine at all.
> 
> This is how the knav stuff ended up. Well it is only used by networking
> atm, so it is 'fine' to have custom API, but it is not portable.

I totally agree generic APIs are better, but not everybody has the resources
to rewrite the whole framework just because they want to do this tiny thing
that isn't covered by the framework yet. In that case it is better to go
with a custom API (that might evolve into a generic API), rather than
overloading the generic API and putting a strain on everybody who works on
the generic API.

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Lars-Peter Clausen]

On 04/17/2018 04:53 PM, Peter Ujfalusi wrote:
> On 2018-04-17 16:58, Lars-Peter Clausen wrote:
 There are two options.

 Either you extend the generic interfaces so it can cover your usecase in a
 generic way. E.g. the ability to attach meta data to transfer.
>>>
>>> Fwiw I have this patch as part of a bigger work to achieve similar results:
>>
>> That's good stuff. Is this in a public tree somewhere?
> 
> Not atm. I can not send the user of the new API and I did not wanted to
> send something like this out of the blue w/o context.
> 
> But as it is a generic patch, I can send it as well. The only thing is
> that the need for the memcpy, so I might end up with
> ptr = get_metadata_ptr(desc, ); /* size: in RX the valid size */
> 
> and set_metadata_size(); /* in TX to tell how the client placed */
> 
> Or something like that, the attach_metadata() as it is works just fine,
> but high throughput might not like the memcpy.
> 

In the most abstracted way I'd say metadata and data are two different data
streams that are correlated and send/received at the same time.

Think multi-planar transfer, like for audio when the right and left channel
are in separate buffers and not interleaved. Or video with different
color/luminance components in separate buffers. This is something that is at
the moment not covered by the dmaengine API either.

 Or you can implement a interface that is specific to your DMA controller 
 and
 any client using this interface knows it is talking to your DMA controller.
>>>
>>> Hrm, so we can have DMA driver specific calls? The reason why TI's keystone 
>>> 2
>>> navigator DMA support was rejected that it was introducing NAV specific 
>>> calls
>>> for clients to configure features not yet supported by the framework.
>>
>> In my opinion it is OK, somebody else might have different ideas. I mean it
>> is not nice, but it is better than the alternative of overloading the
>> generic API with driver specific semantics or introducing some kind of IOCTL
>> catch all callback.
> 
> True, but the generic API can be extended as well to cover new grounds,
> features. Like this metadata thing.
> 
>> If there is tight coupling between the DMA core and client and there is no
>> intention of using a generic client the best solution might even be to no
>> use DMAengine at all.
> 
> This is how the knav stuff ended up. Well it is only used by networking
> atm, so it is 'fine' to have custom API, but it is not portable.

I totally agree generic APIs are better, but not everybody has the resources
to rewrite the whole framework just because they want to do this tiny thing
that isn't covered by the framework yet. In that case it is better to go
with a custom API (that might evolve into a generic API), rather than
overloading the generic API and putting a strain on everybody who works on
the generic API.

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Lars-Peter Clausen]

On 04/17/2018 05:42 PM, Vinod Koul wrote:
> On Tue, Apr 17, 2018 at 04:46:43PM +0300, Peter Ujfalusi wrote:
> 
>> @@ -709,6 +709,11 @@ struct dma_filter {
>>   *  be called after period_len bytes have been transferred.
>>   * @device_prep_interleaved_dma: Transfer expression in a generic way.
>>   * @device_prep_dma_imm_data: DMA's 8 byte immediate data to the dst address
>> + * @device_attach_metadata: Some DMA engines can send and receive side band
>> + *  information, commands or parameters which is not transferred within the
>> + *  data stream itself. In such case clients can set the metadata to the
>> + *  given descriptor and it is going to be sent to the peripheral, or in
>> + *  case of DEV_TO_MEM the provided buffer will receive the metadata.
>>   * @device_config: Pushes a new configuration to a channel, return 0 or an 
>> error
>>   *  code
>>   * @device_pause: Pauses any transfer happening on a channel. Returns
>> @@ -796,6 +801,9 @@ struct dma_device {
>>  struct dma_chan *chan, dma_addr_t dst, u64 data,
>>  unsigned long flags);
>>  
>> +int (*device_attach_metadata)(struct dma_async_tx_descriptor *desc,
>> +  void *data, size_t len);
> 
> while i am okay with the concept, I would not want to go again the custom
> pointer route, this is a no-go for me.
> 
> Instead lets add the vendor data, define that explicitly. We can use struct,
> tokens or something else to define these. But lets try to stay away from
> opaque objects please :-)

Well, for all the DMA core cares about the meta-data stream would be as
opaque as the data stream itself. The data is in the end client specific. It
is data that sits somewhere in memory that should be send along with the
actual data to the client.

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Lars-Peter Clausen]

On 04/17/2018 05:42 PM, Vinod Koul wrote:
> On Tue, Apr 17, 2018 at 04:46:43PM +0300, Peter Ujfalusi wrote:
> 
>> @@ -709,6 +709,11 @@ struct dma_filter {
>>   *  be called after period_len bytes have been transferred.
>>   * @device_prep_interleaved_dma: Transfer expression in a generic way.
>>   * @device_prep_dma_imm_data: DMA's 8 byte immediate data to the dst address
>> + * @device_attach_metadata: Some DMA engines can send and receive side band
>> + *  information, commands or parameters which is not transferred within the
>> + *  data stream itself. In such case clients can set the metadata to the
>> + *  given descriptor and it is going to be sent to the peripheral, or in
>> + *  case of DEV_TO_MEM the provided buffer will receive the metadata.
>>   * @device_config: Pushes a new configuration to a channel, return 0 or an 
>> error
>>   *  code
>>   * @device_pause: Pauses any transfer happening on a channel. Returns
>> @@ -796,6 +801,9 @@ struct dma_device {
>>  struct dma_chan *chan, dma_addr_t dst, u64 data,
>>  unsigned long flags);
>>  
>> +int (*device_attach_metadata)(struct dma_async_tx_descriptor *desc,
>> +  void *data, size_t len);
> 
> while i am okay with the concept, I would not want to go again the custom
> pointer route, this is a no-go for me.
> 
> Instead lets add the vendor data, define that explicitly. We can use struct,
> tokens or something else to define these. But lets try to stay away from
> opaque objects please :-)

Well, for all the DMA core cares about the meta-data stream would be as
opaque as the data stream itself. The data is in the end client specific. It
is data that sits somewhere in memory that should be send along with the
actual data to the client.

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Vinod Koul]

On Tue, Apr 17, 2018 at 04:46:43PM +0300, Peter Ujfalusi wrote:

> @@ -709,6 +709,11 @@ struct dma_filter {
>   *   be called after period_len bytes have been transferred.
>   * @device_prep_interleaved_dma: Transfer expression in a generic way.
>   * @device_prep_dma_imm_data: DMA's 8 byte immediate data to the dst address
> + * @device_attach_metadata: Some DMA engines can send and receive side band
> + *   information, commands or parameters which is not transferred within the
> + *   data stream itself. In such case clients can set the metadata to the
> + *   given descriptor and it is going to be sent to the peripheral, or in
> + *   case of DEV_TO_MEM the provided buffer will receive the metadata.
>   * @device_config: Pushes a new configuration to a channel, return 0 or an 
> error
>   *   code
>   * @device_pause: Pauses any transfer happening on a channel. Returns
> @@ -796,6 +801,9 @@ struct dma_device {
>   struct dma_chan *chan, dma_addr_t dst, u64 data,
>   unsigned long flags);
>  
> + int (*device_attach_metadata)(struct dma_async_tx_descriptor *desc,
> +   void *data, size_t len);

while i am okay with the concept, I would not want to go again the custom
pointer route, this is a no-go for me.

Instead lets add the vendor data, define that explicitly. We can use struct,
tokens or something else to define these. But lets try to stay away from
opaque objects please :-)

-- 
~Vinod

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Vinod Koul]

On Tue, Apr 17, 2018 at 04:46:43PM +0300, Peter Ujfalusi wrote:

> @@ -709,6 +709,11 @@ struct dma_filter {
>   *   be called after period_len bytes have been transferred.
>   * @device_prep_interleaved_dma: Transfer expression in a generic way.
>   * @device_prep_dma_imm_data: DMA's 8 byte immediate data to the dst address
> + * @device_attach_metadata: Some DMA engines can send and receive side band
> + *   information, commands or parameters which is not transferred within the
> + *   data stream itself. In such case clients can set the metadata to the
> + *   given descriptor and it is going to be sent to the peripheral, or in
> + *   case of DEV_TO_MEM the provided buffer will receive the metadata.
>   * @device_config: Pushes a new configuration to a channel, return 0 or an 
> error
>   *   code
>   * @device_pause: Pauses any transfer happening on a channel. Returns
> @@ -796,6 +801,9 @@ struct dma_device {
>   struct dma_chan *chan, dma_addr_t dst, u64 data,
>   unsigned long flags);
>  
> + int (*device_attach_metadata)(struct dma_async_tx_descriptor *desc,
> +   void *data, size_t len);

while i am okay with the concept, I would not want to go again the custom
pointer route, this is a no-go for me.

Instead lets add the vendor data, define that explicitly. We can use struct,
tokens or something else to define these. But lets try to stay away from
opaque objects please :-)

-- 
~Vinod

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Peter Ujfalusi]

On 2018-04-17 16:58, Lars-Peter Clausen wrote:
>>> There are two options.
>>>
>>> Either you extend the generic interfaces so it can cover your usecase in a
>>> generic way. E.g. the ability to attach meta data to transfer.
>>
>> Fwiw I have this patch as part of a bigger work to achieve similar results:
> 
> That's good stuff. Is this in a public tree somewhere?

Not atm. I can not send the user of the new API and I did not wanted to
send something like this out of the blue w/o context.

But as it is a generic patch, I can send it as well. The only thing is
that the need for the memcpy, so I might end up with
ptr = get_metadata_ptr(desc, ); /* size: in RX the valid size */

and set_metadata_size(); /* in TX to tell how the client placed */

Or something like that, the attach_metadata() as it is works just fine,
but high throughput might not like the memcpy.

>>> Or you can implement a interface that is specific to your DMA controller and
>>> any client using this interface knows it is talking to your DMA controller.
>>
>> Hrm, so we can have DMA driver specific calls? The reason why TI's keystone 2
>> navigator DMA support was rejected that it was introducing NAV specific calls
>> for clients to configure features not yet supported by the framework.
> 
> In my opinion it is OK, somebody else might have different ideas. I mean it
> is not nice, but it is better than the alternative of overloading the
> generic API with driver specific semantics or introducing some kind of IOCTL
> catch all callback.

True, but the generic API can be extended as well to cover new grounds,
features. Like this metadata thing.

> If there is tight coupling between the DMA core and client and there is no
> intention of using a generic client the best solution might even be to no
> use DMAengine at all.

This is how the knav stuff ended up. Well it is only used by networking
atm, so it is 'fine' to have custom API, but it is not portable.

> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Peter Ujfalusi]

On 2018-04-17 16:58, Lars-Peter Clausen wrote:
>>> There are two options.
>>>
>>> Either you extend the generic interfaces so it can cover your usecase in a
>>> generic way. E.g. the ability to attach meta data to transfer.
>>
>> Fwiw I have this patch as part of a bigger work to achieve similar results:
> 
> That's good stuff. Is this in a public tree somewhere?

Not atm. I can not send the user of the new API and I did not wanted to
send something like this out of the blue w/o context.

But as it is a generic patch, I can send it as well. The only thing is
that the need for the memcpy, so I might end up with
ptr = get_metadata_ptr(desc, ); /* size: in RX the valid size */

and set_metadata_size(); /* in TX to tell how the client placed */

Or something like that, the attach_metadata() as it is works just fine,
but high throughput might not like the memcpy.

>>> Or you can implement a interface that is specific to your DMA controller and
>>> any client using this interface knows it is talking to your DMA controller.
>>
>> Hrm, so we can have DMA driver specific calls? The reason why TI's keystone 2
>> navigator DMA support was rejected that it was introducing NAV specific calls
>> for clients to configure features not yet supported by the framework.
> 
> In my opinion it is OK, somebody else might have different ideas. I mean it
> is not nice, but it is better than the alternative of overloading the
> generic API with driver specific semantics or introducing some kind of IOCTL
> catch all callback.

True, but the generic API can be extended as well to cover new grounds,
features. Like this metadata thing.

> If there is tight coupling between the DMA core and client and there is no
> intention of using a generic client the best solution might even be to no
> use DMAengine at all.

This is how the knav stuff ended up. Well it is only used by networking
atm, so it is 'fine' to have custom API, but it is not portable.

> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Lars-Peter Clausen]

On 04/17/2018 03:46 PM, Peter Ujfalusi wrote:
> On 2018-04-17 15:54, Lars-Peter Clausen wrote:
>> On 04/17/2018 01:43 PM, Radhey Shyam Pandey wrote:
>>> Hi Vinod,
>>>
>>>> -Original Message-
>>>> From: Vinod Koul [mailto:vinod.k...@intel.com]
>>>> Sent: Wednesday, April 11, 2018 2:39 PM
>>>> To: Radhey Shyam Pandey <radh...@xilinx.com>
>>>> Cc: dan.j.willi...@intel.com; michal.si...@xilinx.com; Appana Durga
>>>> Kedareswara Rao <appa...@xilinx.com>; Radhey Shyam Pandey
>>>> <radh...@xilinx.com>; l...@metafoo.de; dmaeng...@vger.kernel.org;
>>>> linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org
>>>> Subject: Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control
>>>> words to netdev dma client
>>>>
>>>> On Mon, Apr 02, 2018 at 04:09:02PM +0530, Radhey Shyam Pandey wrote:
>>>>
>>>>> +
>>>>> + if (chan->xdev->has_axieth_connected) {
>>>>> + seg = list_first_entry(>segments,
>>>>> + struct xilinx_axidma_tx_segment,
>>>> node);
>>>>> + if (cb.callback_param) {
>>>>> + app_w = (u32 *) cb.callback_param;
>>>>
>>>> why are you interpreting callback_param? This is plainly wrong.
>>>> we do not know what is the interpretation of callback_param and it is
>>>> internal to submitter.
>>> In design, if AXI DMA is connected to AXI Ethernet IP there are certain
>>> AXI4-Stream Status fields (RX) that we need to pass to ethernet driver
>>> along with data buffer. An example includes: checksum fields, packet
>>> length etc. To pass these control words there is a structure defined
>>> between dmaengine and client. Before calling the client callback
>>> stream control words are copied to dma client callback_param struct
>>> (only if axieth is connected).
>>>
>>> I understand it's not an ideal way and we shouldn't be interpreting
>>> callback_param but couldn't find any better alternative of passing
>>> custom information from dmaengine to client driver and still be 
>>> aligned to the framework.
>>>
>>>>
>>>> What exactly is the problem you are trying to solve?
>>> As mentioned above we need to pass AXI4-stream words(custom
>>> data) from dmaengine driver to dma client driver(ethernet) for
>>> each DMA descriptor. Current solution populates callback_param
>>> struct (only if axieth is connected). Please let me know if there is
>>> an alternate solution. 
>>
>> The standard interfaces need to work in a way that a client using them does
>> not have to know to which DMA controller it is connected. In a similar way
>> the DMA controller shouldn't have any client specific logic for the generic
>> interfaces. Otherwise there is no point of having a generic interface.
>>
>> There are two options.
>>
>> Either you extend the generic interfaces so it can cover your usecase in a
>> generic way. E.g. the ability to attach meta data to transfer.
> 
> Fwiw I have this patch as part of a bigger work to achieve similar results:

That's good stuff. Is this in a public tree somewhere?

>> Or you can implement a interface that is specific to your DMA controller and
>> any client using this interface knows it is talking to your DMA controller.
> 
> Hrm, so we can have DMA driver specific calls? The reason why TI's keystone 2
> navigator DMA support was rejected that it was introducing NAV specific calls
> for clients to configure features not yet supported by the framework.

In my opinion it is OK, somebody else might have different ideas. I mean it
is not nice, but it is better than the alternative of overloading the
generic API with driver specific semantics or introducing some kind of IOCTL
catch all callback.

If there is tight coupling between the DMA core and client and there is no
intention of using a generic client the best solution might even be to no
use DMAengine at all.

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Lars-Peter Clausen]

On 04/17/2018 03:46 PM, Peter Ujfalusi wrote:
> On 2018-04-17 15:54, Lars-Peter Clausen wrote:
>> On 04/17/2018 01:43 PM, Radhey Shyam Pandey wrote:
>>> Hi Vinod,
>>>
>>>> -Original Message-
>>>> From: Vinod Koul [mailto:vinod.k...@intel.com]
>>>> Sent: Wednesday, April 11, 2018 2:39 PM
>>>> To: Radhey Shyam Pandey 
>>>> Cc: dan.j.willi...@intel.com; michal.si...@xilinx.com; Appana Durga
>>>> Kedareswara Rao ; Radhey Shyam Pandey
>>>> ; l...@metafoo.de; dmaeng...@vger.kernel.org;
>>>> linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org
>>>> Subject: Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control
>>>> words to netdev dma client
>>>>
>>>> On Mon, Apr 02, 2018 at 04:09:02PM +0530, Radhey Shyam Pandey wrote:
>>>>
>>>>> +
>>>>> + if (chan->xdev->has_axieth_connected) {
>>>>> + seg = list_first_entry(>segments,
>>>>> + struct xilinx_axidma_tx_segment,
>>>> node);
>>>>> + if (cb.callback_param) {
>>>>> + app_w = (u32 *) cb.callback_param;
>>>>
>>>> why are you interpreting callback_param? This is plainly wrong.
>>>> we do not know what is the interpretation of callback_param and it is
>>>> internal to submitter.
>>> In design, if AXI DMA is connected to AXI Ethernet IP there are certain
>>> AXI4-Stream Status fields (RX) that we need to pass to ethernet driver
>>> along with data buffer. An example includes: checksum fields, packet
>>> length etc. To pass these control words there is a structure defined
>>> between dmaengine and client. Before calling the client callback
>>> stream control words are copied to dma client callback_param struct
>>> (only if axieth is connected).
>>>
>>> I understand it's not an ideal way and we shouldn't be interpreting
>>> callback_param but couldn't find any better alternative of passing
>>> custom information from dmaengine to client driver and still be 
>>> aligned to the framework.
>>>
>>>>
>>>> What exactly is the problem you are trying to solve?
>>> As mentioned above we need to pass AXI4-stream words(custom
>>> data) from dmaengine driver to dma client driver(ethernet) for
>>> each DMA descriptor. Current solution populates callback_param
>>> struct (only if axieth is connected). Please let me know if there is
>>> an alternate solution. 
>>
>> The standard interfaces need to work in a way that a client using them does
>> not have to know to which DMA controller it is connected. In a similar way
>> the DMA controller shouldn't have any client specific logic for the generic
>> interfaces. Otherwise there is no point of having a generic interface.
>>
>> There are two options.
>>
>> Either you extend the generic interfaces so it can cover your usecase in a
>> generic way. E.g. the ability to attach meta data to transfer.
> 
> Fwiw I have this patch as part of a bigger work to achieve similar results:

That's good stuff. Is this in a public tree somewhere?

>> Or you can implement a interface that is specific to your DMA controller and
>> any client using this interface knows it is talking to your DMA controller.
> 
> Hrm, so we can have DMA driver specific calls? The reason why TI's keystone 2
> navigator DMA support was rejected that it was introducing NAV specific calls
> for clients to configure features not yet supported by the framework.

In my opinion it is OK, somebody else might have different ideas. I mean it
is not nice, but it is better than the alternative of overloading the
generic API with driver specific semantics or introducing some kind of IOCTL
catch all callback.

If there is tight coupling between the DMA core and client and there is no
intention of using a generic client the best solution might even be to no
use DMAengine at all.

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Peter Ujfalusi]

On 2018-04-17 15:54, Lars-Peter Clausen wrote:
> On 04/17/2018 01:43 PM, Radhey Shyam Pandey wrote:
>> Hi Vinod,
>>
>>> -Original Message-
>>> From: Vinod Koul [mailto:vinod.k...@intel.com]
>>> Sent: Wednesday, April 11, 2018 2:39 PM
>>> To: Radhey Shyam Pandey <radh...@xilinx.com>
>>> Cc: dan.j.willi...@intel.com; michal.si...@xilinx.com; Appana Durga
>>> Kedareswara Rao <appa...@xilinx.com>; Radhey Shyam Pandey
>>> <radh...@xilinx.com>; l...@metafoo.de; dmaeng...@vger.kernel.org;
>>> linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org
>>> Subject: Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control
>>> words to netdev dma client
>>>
>>> On Mon, Apr 02, 2018 at 04:09:02PM +0530, Radhey Shyam Pandey wrote:
>>>
>>>> +
>>>> +  if (chan->xdev->has_axieth_connected) {
>>>> +  seg = list_first_entry(>segments,
>>>> +  struct xilinx_axidma_tx_segment,
>>> node);
>>>> +  if (cb.callback_param) {
>>>> +  app_w = (u32 *) cb.callback_param;
>>>
>>> why are you interpreting callback_param? This is plainly wrong.
>>> we do not know what is the interpretation of callback_param and it is
>>> internal to submitter.
>> In design, if AXI DMA is connected to AXI Ethernet IP there are certain
>> AXI4-Stream Status fields (RX) that we need to pass to ethernet driver
>> along with data buffer. An example includes: checksum fields, packet
>> length etc. To pass these control words there is a structure defined
>> between dmaengine and client. Before calling the client callback
>> stream control words are copied to dma client callback_param struct
>> (only if axieth is connected).
>>
>> I understand it's not an ideal way and we shouldn't be interpreting
>> callback_param but couldn't find any better alternative of passing
>> custom information from dmaengine to client driver and still be 
>> aligned to the framework.
>>
>>>
>>> What exactly is the problem you are trying to solve?
>> As mentioned above we need to pass AXI4-stream words(custom
>> data) from dmaengine driver to dma client driver(ethernet) for
>> each DMA descriptor. Current solution populates callback_param
>> struct (only if axieth is connected). Please let me know if there is
>> an alternate solution. 
> 
> The standard interfaces need to work in a way that a client using them does
> not have to know to which DMA controller it is connected. In a similar way
> the DMA controller shouldn't have any client specific logic for the generic
> interfaces. Otherwise there is no point of having a generic interface.
> 
> There are two options.
> 
> Either you extend the generic interfaces so it can cover your usecase in a
> generic way. E.g. the ability to attach meta data to transfer.

Fwiw I have this patch as part of a bigger work to achieve similar results:

commit f7cf442a37618bbd996f2640ececc4a990ea655e
Author: Peter Ujfalusi <peter.ujfal...@ti.com>
Date:   Wed Nov 22 10:21:59 2017 +0200

dmaengine: Add callback to set per descriptor metadata

Some DMA engines can send and receive side band information, commands or
parameters which is not transferred within the data stream itself. In such
case clients can set the metadata to the given descriptor and it is going
to be sent to the peripheral, or in case of DEV_TO_MEM the provided buffer
will receive the metadata.

Signed-off-by: Peter Ujfalusi <peter.ujfal...@ti.com>

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index cc887a6f74ba..fb8e9c92fb01 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -709,6 +709,11 @@ struct dma_filter {
  * be called after period_len bytes have been transferred.
  * @device_prep_interleaved_dma: Transfer expression in a generic way.
  * @device_prep_dma_imm_data: DMA's 8 byte immediate data to the dst address
+ * @device_attach_metadata: Some DMA engines can send and receive side band
+ * information, commands or parameters which is not transferred within the
+ * data stream itself. In such case clients can set the metadata to the
+ * given descriptor and it is going to be sent to the peripheral, or in
+ * case of DEV_TO_MEM the provided buffer will receive the metadata.
  * @device_config: Pushes a new configuration to a channel, return 0 or an 
error
  * code
  * @device_pause: Pauses any transfer happening on a channel. Returns
@@ -796,6 +801,9 @@ struct dma_device {
struct dma_chan *c

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Peter Ujfalusi]

On 2018-04-17 15:54, Lars-Peter Clausen wrote:
> On 04/17/2018 01:43 PM, Radhey Shyam Pandey wrote:
>> Hi Vinod,
>>
>>> -Original Message-
>>> From: Vinod Koul [mailto:vinod.k...@intel.com]
>>> Sent: Wednesday, April 11, 2018 2:39 PM
>>> To: Radhey Shyam Pandey 
>>> Cc: dan.j.willi...@intel.com; michal.si...@xilinx.com; Appana Durga
>>> Kedareswara Rao ; Radhey Shyam Pandey
>>> ; l...@metafoo.de; dmaeng...@vger.kernel.org;
>>> linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org
>>> Subject: Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control
>>> words to netdev dma client
>>>
>>> On Mon, Apr 02, 2018 at 04:09:02PM +0530, Radhey Shyam Pandey wrote:
>>>
>>>> +
>>>> +  if (chan->xdev->has_axieth_connected) {
>>>> +  seg = list_first_entry(>segments,
>>>> +  struct xilinx_axidma_tx_segment,
>>> node);
>>>> +  if (cb.callback_param) {
>>>> +  app_w = (u32 *) cb.callback_param;
>>>
>>> why are you interpreting callback_param? This is plainly wrong.
>>> we do not know what is the interpretation of callback_param and it is
>>> internal to submitter.
>> In design, if AXI DMA is connected to AXI Ethernet IP there are certain
>> AXI4-Stream Status fields (RX) that we need to pass to ethernet driver
>> along with data buffer. An example includes: checksum fields, packet
>> length etc. To pass these control words there is a structure defined
>> between dmaengine and client. Before calling the client callback
>> stream control words are copied to dma client callback_param struct
>> (only if axieth is connected).
>>
>> I understand it's not an ideal way and we shouldn't be interpreting
>> callback_param but couldn't find any better alternative of passing
>> custom information from dmaengine to client driver and still be 
>> aligned to the framework.
>>
>>>
>>> What exactly is the problem you are trying to solve?
>> As mentioned above we need to pass AXI4-stream words(custom
>> data) from dmaengine driver to dma client driver(ethernet) for
>> each DMA descriptor. Current solution populates callback_param
>> struct (only if axieth is connected). Please let me know if there is
>> an alternate solution. 
> 
> The standard interfaces need to work in a way that a client using them does
> not have to know to which DMA controller it is connected. In a similar way
> the DMA controller shouldn't have any client specific logic for the generic
> interfaces. Otherwise there is no point of having a generic interface.
> 
> There are two options.
> 
> Either you extend the generic interfaces so it can cover your usecase in a
> generic way. E.g. the ability to attach meta data to transfer.

Fwiw I have this patch as part of a bigger work to achieve similar results:

commit f7cf442a37618bbd996f2640ececc4a990ea655e
Author: Peter Ujfalusi 
Date:   Wed Nov 22 10:21:59 2017 +0200

dmaengine: Add callback to set per descriptor metadata

Some DMA engines can send and receive side band information, commands or
parameters which is not transferred within the data stream itself. In such
case clients can set the metadata to the given descriptor and it is going
to be sent to the peripheral, or in case of DEV_TO_MEM the provided buffer
will receive the metadata.

Signed-off-by: Peter Ujfalusi 

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index cc887a6f74ba..fb8e9c92fb01 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -709,6 +709,11 @@ struct dma_filter {
  * be called after period_len bytes have been transferred.
  * @device_prep_interleaved_dma: Transfer expression in a generic way.
  * @device_prep_dma_imm_data: DMA's 8 byte immediate data to the dst address
+ * @device_attach_metadata: Some DMA engines can send and receive side band
+ * information, commands or parameters which is not transferred within the
+ * data stream itself. In such case clients can set the metadata to the
+ * given descriptor and it is going to be sent to the peripheral, or in
+ * case of DEV_TO_MEM the provided buffer will receive the metadata.
  * @device_config: Pushes a new configuration to a channel, return 0 or an 
error
  * code
  * @device_pause: Pauses any transfer happening on a channel. Returns
@@ -796,6 +801,9 @@ struct dma_device {
struct dma_chan *chan, dma_addr_t dst, u64 data,
unsigned long flags);
 
+   int (*device_attach_metadata)(struct dma_async_tx_descriptor *desc,
+

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Lars-Peter Clausen]

On 04/17/2018 01:43 PM, Radhey Shyam Pandey wrote:
> Hi Vinod,
> 
>> -Original Message-
>> From: Vinod Koul [mailto:vinod.k...@intel.com]
>> Sent: Wednesday, April 11, 2018 2:39 PM
>> To: Radhey Shyam Pandey <radh...@xilinx.com>
>> Cc: dan.j.willi...@intel.com; michal.si...@xilinx.com; Appana Durga
>> Kedareswara Rao <appa...@xilinx.com>; Radhey Shyam Pandey
>> <radh...@xilinx.com>; l...@metafoo.de; dmaeng...@vger.kernel.org;
>> linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org
>> Subject: Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control
>> words to netdev dma client
>>
>> On Mon, Apr 02, 2018 at 04:09:02PM +0530, Radhey Shyam Pandey wrote:
>>
>>> +
>>> +   if (chan->xdev->has_axieth_connected) {
>>> +   seg = list_first_entry(>segments,
>>> +   struct xilinx_axidma_tx_segment,
>> node);
>>> +   if (cb.callback_param) {
>>> +   app_w = (u32 *) cb.callback_param;
>>
>> why are you interpreting callback_param? This is plainly wrong.
>> we do not know what is the interpretation of callback_param and it is
>> internal to submitter.
> In design, if AXI DMA is connected to AXI Ethernet IP there are certain
> AXI4-Stream Status fields (RX) that we need to pass to ethernet driver
> along with data buffer. An example includes: checksum fields, packet
> length etc. To pass these control words there is a structure defined
> between dmaengine and client. Before calling the client callback
> stream control words are copied to dma client callback_param struct
> (only if axieth is connected).
> 
> I understand it's not an ideal way and we shouldn't be interpreting
> callback_param but couldn't find any better alternative of passing
> custom information from dmaengine to client driver and still be 
> aligned to the framework.
> 
>>
>> What exactly is the problem you are trying to solve?
> As mentioned above we need to pass AXI4-stream words(custom
> data) from dmaengine driver to dma client driver(ethernet) for
> each DMA descriptor. Current solution populates callback_param
> struct (only if axieth is connected). Please let me know if there is
> an alternate solution. 

The standard interfaces need to work in a way that a client using them does
not have to know to which DMA controller it is connected. In a similar way
the DMA controller shouldn't have any client specific logic for the generic
interfaces. Otherwise there is no point of having a generic interface.

There are two options.

Either you extend the generic interfaces so it can cover your usecase in a
generic way. E.g. the ability to attach meta data to transfer.

Or you can implement a interface that is specific to your DMA controller and
any client using this interface knows it is talking to your DMA controller.

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Lars-Peter Clausen]

On 04/17/2018 01:43 PM, Radhey Shyam Pandey wrote:
> Hi Vinod,
> 
>> -Original Message-
>> From: Vinod Koul [mailto:vinod.k...@intel.com]
>> Sent: Wednesday, April 11, 2018 2:39 PM
>> To: Radhey Shyam Pandey 
>> Cc: dan.j.willi...@intel.com; michal.si...@xilinx.com; Appana Durga
>> Kedareswara Rao ; Radhey Shyam Pandey
>> ; l...@metafoo.de; dmaeng...@vger.kernel.org;
>> linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org
>> Subject: Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control
>> words to netdev dma client
>>
>> On Mon, Apr 02, 2018 at 04:09:02PM +0530, Radhey Shyam Pandey wrote:
>>
>>> +
>>> +   if (chan->xdev->has_axieth_connected) {
>>> +   seg = list_first_entry(>segments,
>>> +   struct xilinx_axidma_tx_segment,
>> node);
>>> +   if (cb.callback_param) {
>>> +   app_w = (u32 *) cb.callback_param;
>>
>> why are you interpreting callback_param? This is plainly wrong.
>> we do not know what is the interpretation of callback_param and it is
>> internal to submitter.
> In design, if AXI DMA is connected to AXI Ethernet IP there are certain
> AXI4-Stream Status fields (RX) that we need to pass to ethernet driver
> along with data buffer. An example includes: checksum fields, packet
> length etc. To pass these control words there is a structure defined
> between dmaengine and client. Before calling the client callback
> stream control words are copied to dma client callback_param struct
> (only if axieth is connected).
> 
> I understand it's not an ideal way and we shouldn't be interpreting
> callback_param but couldn't find any better alternative of passing
> custom information from dmaengine to client driver and still be 
> aligned to the framework.
> 
>>
>> What exactly is the problem you are trying to solve?
> As mentioned above we need to pass AXI4-stream words(custom
> data) from dmaengine driver to dma client driver(ethernet) for
> each DMA descriptor. Current solution populates callback_param
> struct (only if axieth is connected). Please let me know if there is
> an alternate solution. 

The standard interfaces need to work in a way that a client using them does
not have to know to which DMA controller it is connected. In a similar way
the DMA controller shouldn't have any client specific logic for the generic
interfaces. Otherwise there is no point of having a generic interface.

There are two options.

Either you extend the generic interfaces so it can cover your usecase in a
generic way. E.g. the ability to attach meta data to transfer.

Or you can implement a interface that is specific to your DMA controller and
any client using this interface knows it is talking to your DMA controller.

RE: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Radhey Shyam Pandey]

Hi Vinod,

> -Original Message-
> From: Vinod Koul [mailto:vinod.k...@intel.com]
> Sent: Wednesday, April 11, 2018 2:39 PM
> To: Radhey Shyam Pandey <radh...@xilinx.com>
> Cc: dan.j.willi...@intel.com; michal.si...@xilinx.com; Appana Durga
> Kedareswara Rao <appa...@xilinx.com>; Radhey Shyam Pandey
> <radh...@xilinx.com>; l...@metafoo.de; dmaeng...@vger.kernel.org;
> linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control
> words to netdev dma client
> 
> On Mon, Apr 02, 2018 at 04:09:02PM +0530, Radhey Shyam Pandey wrote:
> 
> > +
> > +   if (chan->xdev->has_axieth_connected) {
> > +   seg = list_first_entry(>segments,
> > +   struct xilinx_axidma_tx_segment,
> node);
> > +   if (cb.callback_param) {
> > +   app_w = (u32 *) cb.callback_param;
> 
> why are you interpreting callback_param? This is plainly wrong.
> we do not know what is the interpretation of callback_param and it is
> internal to submitter.
In design, if AXI DMA is connected to AXI Ethernet IP there are certain
AXI4-Stream Status fields (RX) that we need to pass to ethernet driver
along with data buffer. An example includes: checksum fields, packet
length etc. To pass these control words there is a structure defined
between dmaengine and client. Before calling the client callback
stream control words are copied to dma client callback_param struct
(only if axieth is connected).

I understand it's not an ideal way and we shouldn't be interpreting
callback_param but couldn't find any better alternative of passing
custom information from dmaengine to client driver and still be 
aligned to the framework.

> 
> What exactly is the problem you are trying to solve?
As mentioned above we need to pass AXI4-stream words(custom
data) from dmaengine driver to dma client driver(ethernet) for
each DMA descriptor. Current solution populates callback_param
struct (only if axieth is connected). Please let me know if there is
an alternate solution. 

> 
> > +   hw = >hw;
> > +   *app_w = hw->status &
> XILINX_DMA_MAX_TRANS_LEN;
> > +   memcpy(app_w, hw->app, sizeof(u32) *
> > +   XILINX_DMA_NUM_APP_WORDS);
> > +   }
> 
> --
> ~Vinod

RE: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Radhey Shyam Pandey]

Hi Vinod,

> -Original Message-
> From: Vinod Koul [mailto:vinod.k...@intel.com]
> Sent: Wednesday, April 11, 2018 2:39 PM
> To: Radhey Shyam Pandey 
> Cc: dan.j.willi...@intel.com; michal.si...@xilinx.com; Appana Durga
> Kedareswara Rao ; Radhey Shyam Pandey
> ; l...@metafoo.de; dmaeng...@vger.kernel.org;
> linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control
> words to netdev dma client
> 
> On Mon, Apr 02, 2018 at 04:09:02PM +0530, Radhey Shyam Pandey wrote:
> 
> > +
> > +   if (chan->xdev->has_axieth_connected) {
> > +   seg = list_first_entry(>segments,
> > +   struct xilinx_axidma_tx_segment,
> node);
> > +   if (cb.callback_param) {
> > +   app_w = (u32 *) cb.callback_param;
> 
> why are you interpreting callback_param? This is plainly wrong.
> we do not know what is the interpretation of callback_param and it is
> internal to submitter.
In design, if AXI DMA is connected to AXI Ethernet IP there are certain
AXI4-Stream Status fields (RX) that we need to pass to ethernet driver
along with data buffer. An example includes: checksum fields, packet
length etc. To pass these control words there is a structure defined
between dmaengine and client. Before calling the client callback
stream control words are copied to dma client callback_param struct
(only if axieth is connected).

I understand it's not an ideal way and we shouldn't be interpreting
callback_param but couldn't find any better alternative of passing
custom information from dmaengine to client driver and still be 
aligned to the framework.

> 
> What exactly is the problem you are trying to solve?
As mentioned above we need to pass AXI4-stream words(custom
data) from dmaengine driver to dma client driver(ethernet) for
each DMA descriptor. Current solution populates callback_param
struct (only if axieth is connected). Please let me know if there is
an alternate solution. 

> 
> > +   hw = >hw;
> > +   *app_w = hw->status &
> XILINX_DMA_MAX_TRANS_LEN;
> > +   memcpy(app_w, hw->app, sizeof(u32) *
> > +   XILINX_DMA_NUM_APP_WORDS);
> > +   }
> 
> --
> ~Vinod

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Vinod Koul]

On Mon, Apr 02, 2018 at 04:09:02PM +0530, Radhey Shyam Pandey wrote:

> +
> + if (chan->xdev->has_axieth_connected) {
> + seg = list_first_entry(>segments,
> + struct xilinx_axidma_tx_segment, node);
> + if (cb.callback_param) {
> + app_w = (u32 *) cb.callback_param;

why are you interpreting callback_param? This is plainly wrong.
we do not know what is the interpretation of callback_param and it is
internal to submitter.

What exactly is the problem you are trying to solve?

> + hw = >hw;
> + *app_w = hw->status & XILINX_DMA_MAX_TRANS_LEN;
> + memcpy(app_w, hw->app, sizeof(u32) *
> + XILINX_DMA_NUM_APP_WORDS);
> + }

-- 
~Vinod

Re: [RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Vinod Koul]

On Mon, Apr 02, 2018 at 04:09:02PM +0530, Radhey Shyam Pandey wrote:

> +
> + if (chan->xdev->has_axieth_connected) {
> + seg = list_first_entry(>segments,
> + struct xilinx_axidma_tx_segment, node);
> + if (cb.callback_param) {
> + app_w = (u32 *) cb.callback_param;

why are you interpreting callback_param? This is plainly wrong.
we do not know what is the interpretation of callback_param and it is
internal to submitter.

What exactly is the problem you are trying to solve?

> + hw = >hw;
> + *app_w = hw->status & XILINX_DMA_MAX_TRANS_LEN;
> + memcpy(app_w, hw->app, sizeof(u32) *
> + XILINX_DMA_NUM_APP_WORDS);
> + }

-- 
~Vinod

[RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Radhey Shyam Pandey]

Read DT property to check if AXI DMA is connected to axithernet.
If connected pass AXI4-Stream control words to netdev dma client.
It is mandatory that netdev dma client reserve initial memory for
max supported control words in callback_param.

Signed-off-by: Radhey Shyam Pandey 
---
 drivers/dma/xilinx/xilinx_dma.c |   35 +++
 1 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 27b5235..16fee30 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -418,6 +418,7 @@ struct xilinx_dma_config {
  * @rxs_clk: DMA s2mm stream clock
  * @nr_channels: Number of channels DMA device supports
  * @chan_id: DMA channel identifier
+ * @has_axieth_connected: AXI DMA connected to AXI ethernet
  */
 struct xilinx_dma_device {
void __iomem *regs;
@@ -437,6 +438,7 @@ struct xilinx_dma_device {
struct clk *rxs_clk;
u32 nr_channels;
u32 chan_id;
+   bool has_axieth_connected;
 };
 
 /* Macros */
@@ -809,7 +811,10 @@ static void xilinx_dma_chan_handle_cyclic(struct 
xilinx_dma_chan *chan,
 static void xilinx_dma_chan_desc_cleanup(struct xilinx_dma_chan *chan)
 {
struct xilinx_dma_tx_descriptor *desc, *next;
+   struct xilinx_axidma_tx_segment *seg;
+   struct xilinx_axidma_desc_hw *hw;
unsigned long flags;
+   u32 *app_w;
 
spin_lock_irqsave(>lock, flags);
 
@@ -821,17 +826,28 @@ static void xilinx_dma_chan_desc_cleanup(struct 
xilinx_dma_chan *chan)
break;
}
 
-   /* Remove from the list of running transactions */
-   list_del(>node);
-
/* Run the link descriptor callback function */
dmaengine_desc_get_callback(>async_tx, );
-   if (dmaengine_desc_callback_valid()) {
-   spin_unlock_irqrestore(>lock, flags);
-   dmaengine_desc_callback_invoke(, NULL);
-   spin_lock_irqsave(>lock, flags);
+
+   if (chan->xdev->has_axieth_connected) {
+   seg = list_first_entry(>segments,
+   struct xilinx_axidma_tx_segment, node);
+   if (cb.callback_param) {
+   app_w = (u32 *) cb.callback_param;
+   hw = >hw;
+   *app_w = hw->status & XILINX_DMA_MAX_TRANS_LEN;
+   memcpy(app_w, hw->app, sizeof(u32) *
+   XILINX_DMA_NUM_APP_WORDS);
+   }
}
 
+   /* Remove from the list of running transactions */
+   list_del(>node);
+
+   spin_unlock_irqrestore(>lock, flags);
+   dmaengine_desc_callback_invoke(, NULL);
+   spin_lock_irqsave(>lock, flags);
+
/* Run any dependencies, then free the descriptor */
dma_run_dependencies(>async_tx);
xilinx_dma_free_tx_descriptor(chan, desc);
@@ -2602,8 +2618,11 @@ static int xilinx_dma_probe(struct platform_device *pdev)
 
/* Retrieve the DMA engine properties from the device tree */
xdev->has_sg = of_property_read_bool(node, "xlnx,include-sg");
-   if (xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA)
+   if (xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
xdev->mcdma = of_property_read_bool(node, "xlnx,mcdma");
+   xdev->has_axieth_connected = of_property_read_bool(node,
+   "xlnx,axieth-connected");
+   }
 
if (xdev->dma_config->dmatype == XDMA_TYPE_VDMA) {
err = of_property_read_u32(node, "xlnx,num-fstores",
-- 
1.7.1

[RFC 2/6] dmaengine: xilinx_dma: Pass AXI4-Stream control words to netdev dma client

[Radhey Shyam Pandey]

Read DT property to check if AXI DMA is connected to axithernet.
If connected pass AXI4-Stream control words to netdev dma client.
It is mandatory that netdev dma client reserve initial memory for
max supported control words in callback_param.

Signed-off-by: Radhey Shyam Pandey 
---
 drivers/dma/xilinx/xilinx_dma.c |   35 +++
 1 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 27b5235..16fee30 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -418,6 +418,7 @@ struct xilinx_dma_config {
  * @rxs_clk: DMA s2mm stream clock
  * @nr_channels: Number of channels DMA device supports
  * @chan_id: DMA channel identifier
+ * @has_axieth_connected: AXI DMA connected to AXI ethernet
  */
 struct xilinx_dma_device {
void __iomem *regs;
@@ -437,6 +438,7 @@ struct xilinx_dma_device {
struct clk *rxs_clk;
u32 nr_channels;
u32 chan_id;
+   bool has_axieth_connected;
 };
 
 /* Macros */
@@ -809,7 +811,10 @@ static void xilinx_dma_chan_handle_cyclic(struct 
xilinx_dma_chan *chan,
 static void xilinx_dma_chan_desc_cleanup(struct xilinx_dma_chan *chan)
 {
struct xilinx_dma_tx_descriptor *desc, *next;
+   struct xilinx_axidma_tx_segment *seg;
+   struct xilinx_axidma_desc_hw *hw;
unsigned long flags;
+   u32 *app_w;
 
spin_lock_irqsave(>lock, flags);
 
@@ -821,17 +826,28 @@ static void xilinx_dma_chan_desc_cleanup(struct 
xilinx_dma_chan *chan)
break;
}
 
-   /* Remove from the list of running transactions */
-   list_del(>node);
-
/* Run the link descriptor callback function */
dmaengine_desc_get_callback(>async_tx, );
-   if (dmaengine_desc_callback_valid()) {
-   spin_unlock_irqrestore(>lock, flags);
-   dmaengine_desc_callback_invoke(, NULL);
-   spin_lock_irqsave(>lock, flags);
+
+   if (chan->xdev->has_axieth_connected) {
+   seg = list_first_entry(>segments,
+   struct xilinx_axidma_tx_segment, node);
+   if (cb.callback_param) {
+   app_w = (u32 *) cb.callback_param;
+   hw = >hw;
+   *app_w = hw->status & XILINX_DMA_MAX_TRANS_LEN;
+   memcpy(app_w, hw->app, sizeof(u32) *
+   XILINX_DMA_NUM_APP_WORDS);
+   }
}
 
+   /* Remove from the list of running transactions */
+   list_del(>node);
+
+   spin_unlock_irqrestore(>lock, flags);
+   dmaengine_desc_callback_invoke(, NULL);
+   spin_lock_irqsave(>lock, flags);
+
/* Run any dependencies, then free the descriptor */
dma_run_dependencies(>async_tx);
xilinx_dma_free_tx_descriptor(chan, desc);
@@ -2602,8 +2618,11 @@ static int xilinx_dma_probe(struct platform_device *pdev)
 
/* Retrieve the DMA engine properties from the device tree */
xdev->has_sg = of_property_read_bool(node, "xlnx,include-sg");
-   if (xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA)
+   if (xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
xdev->mcdma = of_property_read_bool(node, "xlnx,mcdma");
+   xdev->has_axieth_connected = of_property_read_bool(node,
+   "xlnx,axieth-connected");
+   }
 
if (xdev->dma_config->dmatype == XDMA_TYPE_VDMA) {
err = of_property_read_u32(node, "xlnx,num-fstores",
-- 
1.7.1