[PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[Shawn Guo]

Like dma_set(get)_max_seg_size for max_segment_size, the patch adds
max_segment_number into device_dma_parameters and creates the
corresponding dmaengine API dma_set(get)_max_seg_number for it.

Here is the user story that tells the need of the new api.  The
mxs-mmc is the mmc host controller for Freescale MXS architecture.
There are a pair of  mmc host specific parameters max_seg_size and
max_segs that mxs-mmc host driver needs to tell mmc core, so that
mmc core can know how big each data segment could be and how many
segments could be handled one time in a scatter list by host driver.

The mxs-mmc driver is one user of dmaengine mxs-dma, and it will call
mxs-dma to transfer data in scatter list.  That is to say mxs-mmc has
no idea of what max_seg_size and max_segs should be, because they are
all mxs-dma capability parameters, and mxs-mmc needs to query them
from mxs-dma.

Right now, there is well defined dma api (dma_get_max_seg_size) for
mmc to query max_seg_size from dma driver, but the one for max_segs
is missing.  That's why mxs-mmc driver has to hard-code it.

The mxs-mmc is just one example to demonstrate the need of the new
api, and there are other mmc host drivers (mxcmmc on imx-dma is
another example) and possibly even other dmaengine users need this
new api to know the maximum segments that dma driver can handle per
dma call.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
Changes since v1:
 * Update commit message to explain why the new api is needed

 include/linux/device.h      |    1 +
 include/linux/dma-mapping.h |   15 +++++++++++++++
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/include/linux/device.h b/include/linux/device.h
index c66111a..44cb2528 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -487,6 +487,7 @@ struct device_dma_parameters {
 	 * sg limitations.
 	 */
 	unsigned int max_segment_size;
+	unsigned int max_segment_number;
 	unsigned long segment_boundary_mask;
 };
 
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index ba8319a..fd314f4 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -131,6 +131,21 @@ static inline unsigned int dma_set_max_seg_size(struct device *dev,
 		return -EIO;
 }
 
+static inline unsigned int dma_get_max_seg_number(struct device *dev)
+{
+	return dev->dma_parms ? dev->dma_parms->max_segment_number : 1;
+}
+
+static inline unsigned int dma_set_max_seg_number(struct device *dev,
+						  unsigned int number)
+{
+	if (dev->dma_parms) {
+		dev->dma_parms->max_segment_number = number;
+		return 0;
+	} else
+		return -EIO;
+}
+
 static inline unsigned long dma_get_seg_boundary(struct device *dev)
 {
 	return dev->dma_parms ?
-- 
1.7.4.1

[PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[Shawn Guo]

Like dma_set(get)_max_seg_size for max_segment_size, the patch adds
max_segment_number into device_dma_parameters and creates the
corresponding dmaengine API dma_set(get)_max_seg_number for it.

Here is the user story that tells the need of the new api.  The
mxs-mmc is the mmc host controller for Freescale MXS architecture.
There are a pair of  mmc host specific parameters max_seg_size and
max_segs that mxs-mmc host driver needs to tell mmc core, so that
mmc core can know how big each data segment could be and how many
segments could be handled one time in a scatter list by host driver.

The mxs-mmc driver is one user of dmaengine mxs-dma, and it will call
mxs-dma to transfer data in scatter list.  That is to say mxs-mmc has
no idea of what max_seg_size and max_segs should be, because they are
all mxs-dma capability parameters, and mxs-mmc needs to query them
from mxs-dma.

Right now, there is well defined dma api (dma_get_max_seg_size) for
mmc to query max_seg_size from dma driver, but the one for max_segs
is missing.  That's why mxs-mmc driver has to hard-code it.

The mxs-mmc is just one example to demonstrate the need of the new
api, and there are other mmc host drivers (mxcmmc on imx-dma is
another example) and possibly even other dmaengine users need this
new api to know the maximum segments that dma driver can handle per
dma call.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
Changes since v1:
 * Update commit message to explain why the new api is needed

 include/linux/device.h      |    1 +
 include/linux/dma-mapping.h |   15 +++++++++++++++
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/include/linux/device.h b/include/linux/device.h
index c66111a..44cb2528 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -487,6 +487,7 @@ struct device_dma_parameters {
 	 * sg limitations.
 	 */
 	unsigned int max_segment_size;
+	unsigned int max_segment_number;
 	unsigned long segment_boundary_mask;
 };
 
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index ba8319a..fd314f4 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -131,6 +131,21 @@ static inline unsigned int dma_set_max_seg_size(struct device *dev,
 		return -EIO;
 }
 
+static inline unsigned int dma_get_max_seg_number(struct device *dev)
+{
+	return dev->dma_parms ? dev->dma_parms->max_segment_number : 1;
+}
+
+static inline unsigned int dma_set_max_seg_number(struct device *dev,
+						  unsigned int number)
+{
+	if (dev->dma_parms) {
+		dev->dma_parms->max_segment_number = number;
+		return 0;
+	} else
+		return -EIO;
+}
+
 static inline unsigned long dma_get_seg_boundary(struct device *dev)
 {
 	return dev->dma_parms ?
-- 
1.7.4.1

[PATCH v2 2/3] dmaengine: mxs-dma: set up max_segment_number

[Shawn Guo]

It calls dmaengine API dma_set_max_seg_number to set
device_dma_parameters max_segment_number.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/dma/mxs-dma.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
index 88aad4f..875d8f6 100644
--- a/drivers/dma/mxs-dma.c
+++ b/drivers/dma/mxs-dma.c
@@ -670,6 +670,7 @@ static int __init mxs_dma_probe(struct platform_device *pdev)
 	/* mxs_dma gets 65535 bytes maximum sg size */
 	mxs_dma->dma_device.dev->dma_parms = &mxs_dma->dma_parms;
 	dma_set_max_seg_size(mxs_dma->dma_device.dev, MAX_XFER_BYTES);
+	dma_set_max_seg_number(mxs_dma->dma_device.dev, NUM_CCW);
 
 	mxs_dma->dma_device.device_alloc_chan_resources = mxs_dma_alloc_chan_resources;
 	mxs_dma->dma_device.device_free_chan_resources = mxs_dma_free_chan_resources;
-- 
1.7.4.1

[PATCH v2 2/3] dmaengine: mxs-dma: set up max_segment_number

[Shawn Guo]

It calls dmaengine API dma_set_max_seg_number to set
device_dma_parameters max_segment_number.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/dma/mxs-dma.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
index 88aad4f..875d8f6 100644
--- a/drivers/dma/mxs-dma.c
+++ b/drivers/dma/mxs-dma.c
@@ -670,6 +670,7 @@ static int __init mxs_dma_probe(struct platform_device *pdev)
 	/* mxs_dma gets 65535 bytes maximum sg size */
 	mxs_dma->dma_device.dev->dma_parms = &mxs_dma->dma_parms;
 	dma_set_max_seg_size(mxs_dma->dma_device.dev, MAX_XFER_BYTES);
+	dma_set_max_seg_number(mxs_dma->dma_device.dev, NUM_CCW);
 
 	mxs_dma->dma_device.device_alloc_chan_resources = mxs_dma_alloc_chan_resources;
 	mxs_dma->dma_device.device_free_chan_resources = mxs_dma_free_chan_resources;
-- 
1.7.4.1

[PATCH v2 3/3] mmc: mxs-mmc: call dmaengine API to set mmc->max_segs

[Shawn Guo]

With dmaengine API dma_get_max_seg_number added, we now can call
the API to get and number and then set mmc->max_segs instead of
hard-coding it.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/mmc/host/mxs-mmc.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
index 99d39a6..d783af3 100644
--- a/drivers/mmc/host/mxs-mmc.c
+++ b/drivers/mmc/host/mxs-mmc.c
@@ -745,11 +745,15 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 	mmc->f_max = 288000000;
 	mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
 
-	mmc->max_segs = 52;
 	mmc->max_blk_size = 1 << 0xf;
 	mmc->max_blk_count = (ssp_is_old()) ? 0xff : 0xffffff;
 	mmc->max_req_size = (ssp_is_old()) ? 0xffff : 0xffffffff;
 	mmc->max_seg_size = dma_get_max_seg_size(host->dmach->device->dev);
+	/*
+	 * Reserve one segment for carrying on pio words to get dma engine
+	 * program mmc controller registers
+	 */
+	mmc->max_segs = dma_get_max_seg_number(host->dmach->device->dev) - 1;
 
 	platform_set_drvdata(pdev, mmc);
 
-- 
1.7.4.1

[PATCH v2 3/3] mmc: mxs-mmc: call dmaengine API to set mmc->max_segs

[Shawn Guo]

With dmaengine API dma_get_max_seg_number added, we now can call
the API to get and number and then set mmc->max_segs instead of
hard-coding it.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 drivers/mmc/host/mxs-mmc.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
index 99d39a6..d783af3 100644
--- a/drivers/mmc/host/mxs-mmc.c
+++ b/drivers/mmc/host/mxs-mmc.c
@@ -745,11 +745,15 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 	mmc->f_max = 288000000;
 	mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
 
-	mmc->max_segs = 52;
 	mmc->max_blk_size = 1 << 0xf;
 	mmc->max_blk_count = (ssp_is_old()) ? 0xff : 0xffffff;
 	mmc->max_req_size = (ssp_is_old()) ? 0xffff : 0xffffffff;
 	mmc->max_seg_size = dma_get_max_seg_size(host->dmach->device->dev);
+	/*
+	 * Reserve one segment for carrying on pio words to get dma engine
+	 * program mmc controller registers
+	 */
+	mmc->max_segs = dma_get_max_seg_number(host->dmach->device->dev) - 1;
 
 	platform_set_drvdata(pdev, mmc);
 
-- 
1.7.4.1

Re: [PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[FUJITA Tomonori]

On Mon,  6 Jun 2011 15:30:12 +0800
Shawn Guo <shawn.guo@linaro.org> wrote:

> Like dma_set(get)_max_seg_size for max_segment_size, the patch adds
> max_segment_number into device_dma_parameters and creates the
> corresponding dmaengine API dma_set(get)_max_seg_number for it.

As I wrote in another mail, dma_get|set_max_seg_size and
dma_get|set_seg_boundary are added to the dma mapping API. That is,
they enable the drivers (or subsystems) to tell IOMMUs about the
device dma limitations. When IOMMUs merge scatter list entries, they
look at the limitations.


> Here is the user story that tells the need of the new api.  The
> mxs-mmc is the mmc host controller for Freescale MXS architecture.
> There are a pair of  mmc host specific parameters max_seg_size and
> max_segs that mxs-mmc host driver needs to tell mmc core, so that
> mmc core can know how big each data segment could be and how many
> segments could be handled one time in a scatter list by host driver.
> 
> The mxs-mmc driver is one user of dmaengine mxs-dma, and it will call
> mxs-dma to transfer data in scatter list.  That is to say mxs-mmc has
> no idea of what max_seg_size and max_segs should be, because they are
> all mxs-dma capability parameters, and mxs-mmc needs to query them
> from mxs-dma.

max_segs isn't unrelated with the dma mapping API. I explained above,
IOMMUs doesn't increase the number of segments (could decrease the
number of segments by merging).

The limitation about the number of segment already lives elsewhere
(e.g. queue's limits.max_segments).

[PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[FUJITA Tomonori]

On Mon,  6 Jun 2011 15:30:12 +0800
Shawn Guo <shawn.guo@linaro.org> wrote:

> Like dma_set(get)_max_seg_size for max_segment_size, the patch adds
> max_segment_number into device_dma_parameters and creates the
> corresponding dmaengine API dma_set(get)_max_seg_number for it.

As I wrote in another mail, dma_get|set_max_seg_size and
dma_get|set_seg_boundary are added to the dma mapping API. That is,
they enable the drivers (or subsystems) to tell IOMMUs about the
device dma limitations. When IOMMUs merge scatter list entries, they
look at the limitations.


> Here is the user story that tells the need of the new api.  The
> mxs-mmc is the mmc host controller for Freescale MXS architecture.
> There are a pair of  mmc host specific parameters max_seg_size and
> max_segs that mxs-mmc host driver needs to tell mmc core, so that
> mmc core can know how big each data segment could be and how many
> segments could be handled one time in a scatter list by host driver.
> 
> The mxs-mmc driver is one user of dmaengine mxs-dma, and it will call
> mxs-dma to transfer data in scatter list.  That is to say mxs-mmc has
> no idea of what max_seg_size and max_segs should be, because they are
> all mxs-dma capability parameters, and mxs-mmc needs to query them
> from mxs-dma.

max_segs isn't unrelated with the dma mapping API. I explained above,
IOMMUs doesn't increase the number of segments (could decrease the
number of segments by merging).

The limitation about the number of segment already lives elsewhere
(e.g. queue's limits.max_segments).

Re: [PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[Russell King - ARM Linux]

On Mon, Jun 06, 2011 at 05:06:03PM +0900, FUJITA Tomonori wrote:
> max_segs isn't unrelated with the dma mapping API. I explained above,
> IOMMUs doesn't increase the number of segments (could decrease the
> number of segments by merging).
> 
> The limitation about the number of segment already lives elsewhere
> (e.g. queue's limits.max_segments).

I think you're missing the point entirely.

Lets take the problem at hand: you have two devices.  One of them is
handled by the DMA engine code.  One of them is a block device.

The block layer needs to know the various parameters of what is
allowable for DMA, including such things as the maximum size of a
segment, and the _number_ of segments that can be placed into any
one request.

As the DMA provider is _entirely_ separate and unknown to the block
device driver, the block device driver has no way to sanely provide
these parameters to the block layer - they are not a property of the
block device driver, but of the DMA provider.

However, the DMA provider can't know that it'll be interacting with
the block layer, so there's no way for the DMA provider to tell the
block layer about its parameters.

The only way this can happen is for there to be some way for the DMA
provider to export this information to the block device driver, so
the block device driver can then inform its upper layers what the DMA
capabilities are.

So to say that "it lives elsewhere" is completely missing the problem
trying to be addressed by these patches.

[PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[Russell King - ARM Linux]

On Mon, Jun 06, 2011 at 05:06:03PM +0900, FUJITA Tomonori wrote:
> max_segs isn't unrelated with the dma mapping API. I explained above,
> IOMMUs doesn't increase the number of segments (could decrease the
> number of segments by merging).
> 
> The limitation about the number of segment already lives elsewhere
> (e.g. queue's limits.max_segments).

I think you're missing the point entirely.

Lets take the problem at hand: you have two devices.  One of them is
handled by the DMA engine code.  One of them is a block device.

The block layer needs to know the various parameters of what is
allowable for DMA, including such things as the maximum size of a
segment, and the _number_ of segments that can be placed into any
one request.

As the DMA provider is _entirely_ separate and unknown to the block
device driver, the block device driver has no way to sanely provide
these parameters to the block layer - they are not a property of the
block device driver, but of the DMA provider.

However, the DMA provider can't know that it'll be interacting with
the block layer, so there's no way for the DMA provider to tell the
block layer about its parameters.

The only way this can happen is for there to be some way for the DMA
provider to export this information to the block device driver, so
the block device driver can then inform its upper layers what the DMA
capabilities are.

So to say that "it lives elsewhere" is completely missing the problem
trying to be addressed by these patches.

Re: [PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[FUJITA Tomonori]

On Mon, 6 Jun 2011 10:14:10 +0100
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> On Mon, Jun 06, 2011 at 05:06:03PM +0900, FUJITA Tomonori wrote:
> > max_segs isn't unrelated with the dma mapping API. I explained above,
> > IOMMUs doesn't increase the number of segments (could decrease the
> > number of segments by merging).
> > 
> > The limitation about the number of segment already lives elsewhere
> > (e.g. queue's limits.max_segments).
> 
> I think you're missing the point entirely.
> 
> Lets take the problem at hand: you have two devices.  One of them is
> handled by the DMA engine code.  One of them is a block device.
> 
> The block layer needs to know the various parameters of what is
> allowable for DMA, including such things as the maximum size of a
> segment, and the _number_ of segments that can be placed into any
> one request.
> 
> As the DMA provider is _entirely_ separate and unknown to the block
> device driver, the block device driver has no way to sanely provide
> these parameters to the block layer - they are not a property of the
> block device driver, but of the DMA provider.

struct device_dma_parameters is used for a property of the block
device drivers (and scsi HBA drivers, etc). Not DMA provider. Right?

The drivers calls dma_set_seg_boundary() and the subsystems call
dma_get_seg_boundary to set the value to queues.

This patch is trying to use struct device_dma_parameters in a
different way. It adds a new DMA parameter but for the DMA parameter
for a different layer. I'm not sure about different-layer stuff in 
one structure and using similar API.

[PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[FUJITA Tomonori]

On Mon, 6 Jun 2011 10:14:10 +0100
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> On Mon, Jun 06, 2011 at 05:06:03PM +0900, FUJITA Tomonori wrote:
> > max_segs isn't unrelated with the dma mapping API. I explained above,
> > IOMMUs doesn't increase the number of segments (could decrease the
> > number of segments by merging).
> > 
> > The limitation about the number of segment already lives elsewhere
> > (e.g. queue's limits.max_segments).
> 
> I think you're missing the point entirely.
> 
> Lets take the problem at hand: you have two devices.  One of them is
> handled by the DMA engine code.  One of them is a block device.
> 
> The block layer needs to know the various parameters of what is
> allowable for DMA, including such things as the maximum size of a
> segment, and the _number_ of segments that can be placed into any
> one request.
> 
> As the DMA provider is _entirely_ separate and unknown to the block
> device driver, the block device driver has no way to sanely provide
> these parameters to the block layer - they are not a property of the
> block device driver, but of the DMA provider.

struct device_dma_parameters is used for a property of the block
device drivers (and scsi HBA drivers, etc). Not DMA provider. Right?

The drivers calls dma_set_seg_boundary() and the subsystems call
dma_get_seg_boundary to set the value to queues.

This patch is trying to use struct device_dma_parameters in a
different way. It adds a new DMA parameter but for the DMA parameter
for a different layer. I'm not sure about different-layer stuff in 
one structure and using similar API.

Re: [PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[Russell King - ARM Linux]

On Mon, Jun 06, 2011 at 06:41:09PM +0900, FUJITA Tomonori wrote:
> On Mon, 6 Jun 2011 10:14:10 +0100
> Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> 
> > On Mon, Jun 06, 2011 at 05:06:03PM +0900, FUJITA Tomonori wrote:
> > > max_segs isn't unrelated with the dma mapping API. I explained above,
> > > IOMMUs doesn't increase the number of segments (could decrease the
> > > number of segments by merging).
> > > 
> > > The limitation about the number of segment already lives elsewhere
> > > (e.g. queue's limits.max_segments).
> > 
> > I think you're missing the point entirely.
> > 
> > Lets take the problem at hand: you have two devices.  One of them is
> > handled by the DMA engine code.  One of them is a block device.
> > 
> > The block layer needs to know the various parameters of what is
> > allowable for DMA, including such things as the maximum size of a
> > segment, and the _number_ of segments that can be placed into any
> > one request.
> > 
> > As the DMA provider is _entirely_ separate and unknown to the block
> > device driver, the block device driver has no way to sanely provide
> > these parameters to the block layer - they are not a property of the
> > block device driver, but of the DMA provider.
> 
> struct device_dma_parameters is used for a property of the block
> device drivers (and scsi HBA drivers, etc). Not DMA provider. Right?

Wrong.  struct device_dma_parameters is a property of the _DMA_ _provider_.
It has to be.  Read what I said above and think about it.

In many cases, it so happens that the DMA provider and the block device
driver are the same entity, and so it may appear that device_dma_parameters
is a property of the block device driver.  As soon as you have to start
dealing with DMA providers being separate from the block device driver
then your eyes will be opened and you'll see that it can't work the way
you seem to want it to.

The DMA parameters have to come from the DMA provider or they're a total
nonsense.

> The drivers calls dma_set_seg_boundary() and the subsystems call
> dma_get_seg_boundary to set the value to queues.

And the device parameter which you pass into that has to be the DMA
providers struct device, not the block device's struct device.  The
block device itself has no DMA parameters of its own.

> This patch is trying to use struct device_dma_parameters in a
> different way. It adds a new DMA parameter but for the DMA parameter
> for a different layer. I'm not sure about different-layer stuff in 
> one structure and using similar API.

No it's not.

[PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[Russell King - ARM Linux]

On Mon, Jun 06, 2011 at 06:41:09PM +0900, FUJITA Tomonori wrote:
> On Mon, 6 Jun 2011 10:14:10 +0100
> Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> 
> > On Mon, Jun 06, 2011 at 05:06:03PM +0900, FUJITA Tomonori wrote:
> > > max_segs isn't unrelated with the dma mapping API. I explained above,
> > > IOMMUs doesn't increase the number of segments (could decrease the
> > > number of segments by merging).
> > > 
> > > The limitation about the number of segment already lives elsewhere
> > > (e.g. queue's limits.max_segments).
> > 
> > I think you're missing the point entirely.
> > 
> > Lets take the problem at hand: you have two devices.  One of them is
> > handled by the DMA engine code.  One of them is a block device.
> > 
> > The block layer needs to know the various parameters of what is
> > allowable for DMA, including such things as the maximum size of a
> > segment, and the _number_ of segments that can be placed into any
> > one request.
> > 
> > As the DMA provider is _entirely_ separate and unknown to the block
> > device driver, the block device driver has no way to sanely provide
> > these parameters to the block layer - they are not a property of the
> > block device driver, but of the DMA provider.
> 
> struct device_dma_parameters is used for a property of the block
> device drivers (and scsi HBA drivers, etc). Not DMA provider. Right?

Wrong.  struct device_dma_parameters is a property of the _DMA_ _provider_.
It has to be.  Read what I said above and think about it.

In many cases, it so happens that the DMA provider and the block device
driver are the same entity, and so it may appear that device_dma_parameters
is a property of the block device driver.  As soon as you have to start
dealing with DMA providers being separate from the block device driver
then your eyes will be opened and you'll see that it can't work the way
you seem to want it to.

The DMA parameters have to come from the DMA provider or they're a total
nonsense.

> The drivers calls dma_set_seg_boundary() and the subsystems call
> dma_get_seg_boundary to set the value to queues.

And the device parameter which you pass into that has to be the DMA
providers struct device, not the block device's struct device.  The
block device itself has no DMA parameters of its own.

> This patch is trying to use struct device_dma_parameters in a
> different way. It adds a new DMA parameter but for the DMA parameter
> for a different layer. I'm not sure about different-layer stuff in 
> one structure and using similar API.

No it's not.

Re: [PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[FUJITA Tomonori]

On Mon, 6 Jun 2011 10:47:51 +0100
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> On Mon, Jun 06, 2011 at 06:41:09PM +0900, FUJITA Tomonori wrote:
> > On Mon, 6 Jun 2011 10:14:10 +0100
> > Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> > 
> > > On Mon, Jun 06, 2011 at 05:06:03PM +0900, FUJITA Tomonori wrote:
> > > > max_segs isn't unrelated with the dma mapping API. I explained above,
> > > > IOMMUs doesn't increase the number of segments (could decrease the
> > > > number of segments by merging).
> > > > 
> > > > The limitation about the number of segment already lives elsewhere
> > > > (e.g. queue's limits.max_segments).
> > > 
> > > I think you're missing the point entirely.
> > > 
> > > Lets take the problem at hand: you have two devices.  One of them is
> > > handled by the DMA engine code.  One of them is a block device.
> > > 
> > > The block layer needs to know the various parameters of what is
> > > allowable for DMA, including such things as the maximum size of a
> > > segment, and the _number_ of segments that can be placed into any
> > > one request.
> > > 
> > > As the DMA provider is _entirely_ separate and unknown to the block
> > > device driver, the block device driver has no way to sanely provide
> > > these parameters to the block layer - they are not a property of the
> > > block device driver, but of the DMA provider.
> > 
> > struct device_dma_parameters is used for a property of the block
> > device drivers (and scsi HBA drivers, etc). Not DMA provider. Right?
> 
> Wrong.  struct device_dma_parameters is a property of the _DMA_ _provider_.
> It has to be.  Read what I said above and think about it.

I think that it's up to your definition of DMA provider.

> In many cases, it so happens that the DMA provider and the block device
> driver are the same entity, and so it may appear that device_dma_parameters

But could be the different entities, right? If so, the value should be
smaller one? Who is responsible for setting the correct value? The
proposed API blindly set the value (just overwrite). The API would be
better to set a new value only when the new value is smaller? Or
having a separate structure and selecting the smallest value?

struct device_dma_parameters assumes that the DMA provider and the
block (and SCSI, etc) device driver are the same entity.

> is a property of the block device driver.  As soon as you have to start
> dealing with DMA providers being separate from the block device driver
> then your eyes will be opened and you'll see that it can't work the way
> you seem to want it to.
> 
> The DMA parameters have to come from the DMA provider or they're a total
> nonsense.

I don't think that I claim that the DMA parameters don't come from the
DMA provider. It depends on the definition of the DMA provider,
though.

[PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[FUJITA Tomonori]

On Mon, 6 Jun 2011 10:47:51 +0100
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> On Mon, Jun 06, 2011 at 06:41:09PM +0900, FUJITA Tomonori wrote:
> > On Mon, 6 Jun 2011 10:14:10 +0100
> > Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> > 
> > > On Mon, Jun 06, 2011 at 05:06:03PM +0900, FUJITA Tomonori wrote:
> > > > max_segs isn't unrelated with the dma mapping API. I explained above,
> > > > IOMMUs doesn't increase the number of segments (could decrease the
> > > > number of segments by merging).
> > > > 
> > > > The limitation about the number of segment already lives elsewhere
> > > > (e.g. queue's limits.max_segments).
> > > 
> > > I think you're missing the point entirely.
> > > 
> > > Lets take the problem at hand: you have two devices.  One of them is
> > > handled by the DMA engine code.  One of them is a block device.
> > > 
> > > The block layer needs to know the various parameters of what is
> > > allowable for DMA, including such things as the maximum size of a
> > > segment, and the _number_ of segments that can be placed into any
> > > one request.
> > > 
> > > As the DMA provider is _entirely_ separate and unknown to the block
> > > device driver, the block device driver has no way to sanely provide
> > > these parameters to the block layer - they are not a property of the
> > > block device driver, but of the DMA provider.
> > 
> > struct device_dma_parameters is used for a property of the block
> > device drivers (and scsi HBA drivers, etc). Not DMA provider. Right?
> 
> Wrong.  struct device_dma_parameters is a property of the _DMA_ _provider_.
> It has to be.  Read what I said above and think about it.

I think that it's up to your definition of DMA provider.

> In many cases, it so happens that the DMA provider and the block device
> driver are the same entity, and so it may appear that device_dma_parameters

But could be the different entities, right? If so, the value should be
smaller one? Who is responsible for setting the correct value? The
proposed API blindly set the value (just overwrite). The API would be
better to set a new value only when the new value is smaller? Or
having a separate structure and selecting the smallest value?

struct device_dma_parameters assumes that the DMA provider and the
block (and SCSI, etc) device driver are the same entity.

> is a property of the block device driver.  As soon as you have to start
> dealing with DMA providers being separate from the block device driver
> then your eyes will be opened and you'll see that it can't work the way
> you seem to want it to.
> 
> The DMA parameters have to come from the DMA provider or they're a total
> nonsense.

I don't think that I claim that the DMA parameters don't come from the
DMA provider. It depends on the definition of the DMA provider,
though.

Re: [PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[Russell King - ARM Linux]

On Mon, Jun 06, 2011 at 07:12:20PM +0900, FUJITA Tomonori wrote:
> On Mon, 6 Jun 2011 10:47:51 +0100
> Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> 
> > On Mon, Jun 06, 2011 at 06:41:09PM +0900, FUJITA Tomonori wrote:
> > > On Mon, 6 Jun 2011 10:14:10 +0100
> > > Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> > > 
> > > > On Mon, Jun 06, 2011 at 05:06:03PM +0900, FUJITA Tomonori wrote:
> > > > > max_segs isn't unrelated with the dma mapping API. I explained above,
> > > > > IOMMUs doesn't increase the number of segments (could decrease the
> > > > > number of segments by merging).
> > > > > 
> > > > > The limitation about the number of segment already lives elsewhere
> > > > > (e.g. queue's limits.max_segments).
> > > > 
> > > > I think you're missing the point entirely.
> > > > 
> > > > Lets take the problem at hand: you have two devices.  One of them is
> > > > handled by the DMA engine code.  One of them is a block device.
> > > > 
> > > > The block layer needs to know the various parameters of what is
> > > > allowable for DMA, including such things as the maximum size of a
> > > > segment, and the _number_ of segments that can be placed into any
> > > > one request.
> > > > 
> > > > As the DMA provider is _entirely_ separate and unknown to the block
> > > > device driver, the block device driver has no way to sanely provide
> > > > these parameters to the block layer - they are not a property of the
> > > > block device driver, but of the DMA provider.
> > > 
> > > struct device_dma_parameters is used for a property of the block
> > > device drivers (and scsi HBA drivers, etc). Not DMA provider. Right?
> > 
> > Wrong.  struct device_dma_parameters is a property of the _DMA_ _provider_.
> > It has to be.  Read what I said above and think about it.
> 
> I think that it's up to your definition of DMA provider.

I give up.

[PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[Russell King - ARM Linux]

On Mon, Jun 06, 2011 at 07:12:20PM +0900, FUJITA Tomonori wrote:
> On Mon, 6 Jun 2011 10:47:51 +0100
> Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> 
> > On Mon, Jun 06, 2011 at 06:41:09PM +0900, FUJITA Tomonori wrote:
> > > On Mon, 6 Jun 2011 10:14:10 +0100
> > > Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> > > 
> > > > On Mon, Jun 06, 2011 at 05:06:03PM +0900, FUJITA Tomonori wrote:
> > > > > max_segs isn't unrelated with the dma mapping API. I explained above,
> > > > > IOMMUs doesn't increase the number of segments (could decrease the
> > > > > number of segments by merging).
> > > > > 
> > > > > The limitation about the number of segment already lives elsewhere
> > > > > (e.g. queue's limits.max_segments).
> > > > 
> > > > I think you're missing the point entirely.
> > > > 
> > > > Lets take the problem at hand: you have two devices.  One of them is
> > > > handled by the DMA engine code.  One of them is a block device.
> > > > 
> > > > The block layer needs to know the various parameters of what is
> > > > allowable for DMA, including such things as the maximum size of a
> > > > segment, and the _number_ of segments that can be placed into any
> > > > one request.
> > > > 
> > > > As the DMA provider is _entirely_ separate and unknown to the block
> > > > device driver, the block device driver has no way to sanely provide
> > > > these parameters to the block layer - they are not a property of the
> > > > block device driver, but of the DMA provider.
> > > 
> > > struct device_dma_parameters is used for a property of the block
> > > device drivers (and scsi HBA drivers, etc). Not DMA provider. Right?
> > 
> > Wrong.  struct device_dma_parameters is a property of the _DMA_ _provider_.
> > It has to be.  Read what I said above and think about it.
> 
> I think that it's up to your definition of DMA provider.

I give up.

Re: [PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[Dan Williams]

On Mon, Jun 6, 2011 at 3:12 AM, FUJITA Tomonori
<fujita.tomonori@lab.ntt.co.jp> wrote:
> On Mon, 6 Jun 2011 10:47:51 +0100
> Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
>
>> On Mon, Jun 06, 2011 at 06:41:09PM +0900, FUJITA Tomonori wrote:
>> > On Mon, 6 Jun 2011 10:14:10 +0100
>> > Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
>> >
>> > > On Mon, Jun 06, 2011 at 05:06:03PM +0900, FUJITA Tomonori wrote:
>> > > > max_segs isn't unrelated with the dma mapping API. I explained above,
>> > > > IOMMUs doesn't increase the number of segments (could decrease the
>> > > > number of segments by merging).
>> > > >
>> > > > The limitation about the number of segment already lives elsewhere
>> > > > (e.g. queue's limits.max_segments).
>> > >
>> > > I think you're missing the point entirely.
>> > >
>> > > Lets take the problem at hand: you have two devices.  One of them is
>> > > handled by the DMA engine code.  One of them is a block device.
>> > >
>> > > The block layer needs to know the various parameters of what is
>> > > allowable for DMA, including such things as the maximum size of a
>> > > segment, and the _number_ of segments that can be placed into any
>> > > one request.
>> > >
>> > > As the DMA provider is _entirely_ separate and unknown to the block
>> > > device driver, the block device driver has no way to sanely provide
>> > > these parameters to the block layer - they are not a property of the
>> > > block device driver, but of the DMA provider.
>> >
>> > struct device_dma_parameters is used for a property of the block
>> > device drivers (and scsi HBA drivers, etc). Not DMA provider. Right?
>>
>> Wrong.  struct device_dma_parameters is a property of the _DMA_ _provider_.
>> It has to be.  Read what I said above and think about it.
>
> I think that it's up to your definition of DMA provider.
>
>> In many cases, it so happens that the DMA provider and the block device
>> driver are the same entity, and so it may appear that device_dma_parameters
>
> But could be the different entities, right? If so, the value should be
> smaller one? Who is responsible for setting the correct value? The
> proposed API blindly set the value (just overwrite). The API would be
> better to set a new value only when the new value is smaller? Or
> having a separate structure and selecting the smallest value?
>
> struct device_dma_parameters assumes that the DMA provider and the
> block (and SCSI, etc) device driver are the same entity.
>
>> is a property of the block device driver.  As soon as you have to start
>> dealing with DMA providers being separate from the block device driver
>> then your eyes will be opened and you'll see that it can't work the way
>> you seem to want it to.
>>
>> The DMA parameters have to come from the DMA provider or they're a total
>> nonsense.
>
> I don't think that I claim that the DMA parameters don't come from the
> DMA provider. It depends on the definition of the DMA provider,
> though.

dmaengine expands the class of dma providers to include standalone dma
agents on a host bus (or elsewhere) in addition to the traditional bus
mastering host-bus-adapters that the existing api understands.  So in
the case of slave dma the dma capabilities of the block-device are
irrelevant because another agent will do the transfer on behalf of the
block-device driver.  So the value should be whatever the dma device
driver says it is.

--
Dan

[PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[Dan Williams]

On Mon, Jun 6, 2011 at 3:12 AM, FUJITA Tomonori
<fujita.tomonori@lab.ntt.co.jp> wrote:
> On Mon, 6 Jun 2011 10:47:51 +0100
> Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
>
>> On Mon, Jun 06, 2011 at 06:41:09PM +0900, FUJITA Tomonori wrote:
>> > On Mon, 6 Jun 2011 10:14:10 +0100
>> > Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
>> >
>> > > On Mon, Jun 06, 2011 at 05:06:03PM +0900, FUJITA Tomonori wrote:
>> > > > max_segs isn't unrelated with the dma mapping API. I explained above,
>> > > > IOMMUs doesn't increase the number of segments (could decrease the
>> > > > number of segments by merging).
>> > > >
>> > > > The limitation about the number of segment already lives elsewhere
>> > > > (e.g. queue's limits.max_segments).
>> > >
>> > > I think you're missing the point entirely.
>> > >
>> > > Lets take the problem at hand: you have two devices. ?One of them is
>> > > handled by the DMA engine code. ?One of them is a block device.
>> > >
>> > > The block layer needs to know the various parameters of what is
>> > > allowable for DMA, including such things as the maximum size of a
>> > > segment, and the _number_ of segments that can be placed into any
>> > > one request.
>> > >
>> > > As the DMA provider is _entirely_ separate and unknown to the block
>> > > device driver, the block device driver has no way to sanely provide
>> > > these parameters to the block layer - they are not a property of the
>> > > block device driver, but of the DMA provider.
>> >
>> > struct device_dma_parameters is used for a property of the block
>> > device drivers (and scsi HBA drivers, etc). Not DMA provider. Right?
>>
>> Wrong. ?struct device_dma_parameters is a property of the _DMA_ _provider_.
>> It has to be. ?Read what I said above and think about it.
>
> I think that it's up to your definition of DMA provider.
>
>> In many cases, it so happens that the DMA provider and the block device
>> driver are the same entity, and so it may appear that device_dma_parameters
>
> But could be the different entities, right? If so, the value should be
> smaller one? Who is responsible for setting the correct value? The
> proposed API blindly set the value (just overwrite). The API would be
> better to set a new value only when the new value is smaller? Or
> having a separate structure and selecting the smallest value?
>
> struct device_dma_parameters assumes that the DMA provider and the
> block (and SCSI, etc) device driver are the same entity.
>
>> is a property of the block device driver. ?As soon as you have to start
>> dealing with DMA providers being separate from the block device driver
>> then your eyes will be opened and you'll see that it can't work the way
>> you seem to want it to.
>>
>> The DMA parameters have to come from the DMA provider or they're a total
>> nonsense.
>
> I don't think that I claim that the DMA parameters don't come from the
> DMA provider. It depends on the definition of the DMA provider,
> though.

dmaengine expands the class of dma providers to include standalone dma
agents on a host bus (or elsewhere) in addition to the traditional bus
mastering host-bus-adapters that the existing api understands.  So in
the case of slave dma the dma capabilities of the block-device are
irrelevant because another agent will do the transfer on behalf of the
block-device driver.  So the value should be whatever the dma device
driver says it is.

--
Dan

Re: [PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[Dan Williams]

On Mon, Jun 6, 2011 at 12:30 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> Like dma_set(get)_max_seg_size for max_segment_size, the patch adds
> max_segment_number into device_dma_parameters and creates the
> corresponding dmaengine API dma_set(get)_max_seg_number for it.
>
> Here is the user story that tells the need of the new api.  The
> mxs-mmc is the mmc host controller for Freescale MXS architecture.
> There are a pair of  mmc host specific parameters max_seg_size and
> max_segs that mxs-mmc host driver needs to tell mmc core, so that
> mmc core can know how big each data segment could be and how many
> segments could be handled one time in a scatter list by host driver.
>
> The mxs-mmc driver is one user of dmaengine mxs-dma, and it will call
> mxs-dma to transfer data in scatter list.  That is to say mxs-mmc has
> no idea of what max_seg_size and max_segs should be, because they are
> all mxs-dma capability parameters, and mxs-mmc needs to query them
> from mxs-dma.
>
> Right now, there is well defined dma api (dma_get_max_seg_size) for
> mmc to query max_seg_size from dma driver, but the one for max_segs
> is missing.  That's why mxs-mmc driver has to hard-code it.
>
> The mxs-mmc is just one example to demonstrate the need of the new
> api, and there are other mmc host drivers (mxcmmc on imx-dma is
> another example) and possibly even other dmaengine users need this
> new api to know the maximum segments that dma driver can handle per
> dma call.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
> Changes since v1:
>  * Update commit message to explain why the new api is needed
>
>  include/linux/device.h      |    1 +
>  include/linux/dma-mapping.h |   15 +++++++++++++++
>  2 files changed, 16 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/device.h b/include/linux/device.h
> index c66111a..44cb2528 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -487,6 +487,7 @@ struct device_dma_parameters {
>         * sg limitations.
>         */

Given the discussion seems like this patch is missing an update to the
documentation of the struct to clarify the definition of dma provider.
 I.e. that this info belongs to the device doing the dma, and that is
is not necessarily the same as the device that is requesting dma
service.

Other than that seems like a natural extension of the existing usage
of device_dma_parameters in drivers/dma/.

[PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[Dan Williams]

On Mon, Jun 6, 2011 at 12:30 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> Like dma_set(get)_max_seg_size for max_segment_size, the patch adds
> max_segment_number into device_dma_parameters and creates the
> corresponding dmaengine API dma_set(get)_max_seg_number for it.
>
> Here is the user story that tells the need of the new api. ?The
> mxs-mmc is the mmc host controller for Freescale MXS architecture.
> There are a pair of ?mmc host specific parameters max_seg_size and
> max_segs that mxs-mmc host driver needs to tell mmc core, so that
> mmc core can know how big each data segment could be and how many
> segments could be handled one time in a scatter list by host driver.
>
> The mxs-mmc driver is one user of dmaengine mxs-dma, and it will call
> mxs-dma to transfer data in scatter list. ?That is to say mxs-mmc has
> no idea of what max_seg_size and max_segs should be, because they are
> all mxs-dma capability parameters, and mxs-mmc needs to query them
> from mxs-dma.
>
> Right now, there is well defined dma api (dma_get_max_seg_size) for
> mmc to query max_seg_size from dma driver, but the one for max_segs
> is missing. ?That's why mxs-mmc driver has to hard-code it.
>
> The mxs-mmc is just one example to demonstrate the need of the new
> api, and there are other mmc host drivers (mxcmmc on imx-dma is
> another example) and possibly even other dmaengine users need this
> new api to know the maximum segments that dma driver can handle per
> dma call.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
> Changes since v1:
> ?* Update commit message to explain why the new api is needed
>
> ?include/linux/device.h ? ? ?| ? ?1 +
> ?include/linux/dma-mapping.h | ? 15 +++++++++++++++
> ?2 files changed, 16 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/device.h b/include/linux/device.h
> index c66111a..44cb2528 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -487,6 +487,7 @@ struct device_dma_parameters {
> ? ? ? ? * sg limitations.
> ? ? ? ? */

Given the discussion seems like this patch is missing an update to the
documentation of the struct to clarify the definition of dma provider.
 I.e. that this info belongs to the device doing the dma, and that is
is not necessarily the same as the device that is requesting dma
service.

Other than that seems like a natural extension of the existing usage
of device_dma_parameters in drivers/dma/.

Re: [PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[FUJITA Tomonori]

On Mon, 6 Jun 2011 11:48:56 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> dmaengine expands the class of dma providers to include standalone dma
> agents on a host bus (or elsewhere) in addition to the traditional bus
> mastering host-bus-adapters that the existing api understands.  So in
> the case of slave dma the dma capabilities of the block-device are
> irrelevant because another agent will do the transfer on behalf of the
> block-device driver.  So the value should be whatever the dma device
> driver says it is.

The dma parameter restriction could be due to software (HBA drivers,
or subsystem). The value should be whatever the dma device driver says
it is in such case?

[PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[FUJITA Tomonori]

On Mon, 6 Jun 2011 11:48:56 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> dmaengine expands the class of dma providers to include standalone dma
> agents on a host bus (or elsewhere) in addition to the traditional bus
> mastering host-bus-adapters that the existing api understands.  So in
> the case of slave dma the dma capabilities of the block-device are
> irrelevant because another agent will do the transfer on behalf of the
> block-device driver.  So the value should be whatever the dma device
> driver says it is.

The dma parameter restriction could be due to software (HBA drivers,
or subsystem). The value should be whatever the dma device driver says
it is in such case?

Re: [PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[Dan Williams]

On Tue, Jun 7, 2011 at 10:23 PM, FUJITA Tomonori
<fujita.tomonori@lab.ntt.co.jp> wrote:
> On Mon, 6 Jun 2011 11:48:56 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
>
>> dmaengine expands the class of dma providers to include standalone dma
>> agents on a host bus (or elsewhere) in addition to the traditional bus
>> mastering host-bus-adapters that the existing api understands.  So in
>> the case of slave dma the dma capabilities of the block-device are
>> irrelevant because another agent will do the transfer on behalf of the
>> block-device driver.  So the value should be whatever the dma device
>> driver says it is.
>
> The dma parameter restriction could be due to software (HBA drivers,
> or subsystem). The value should be whatever the dma device driver says
> it is in such case?

I'm assuming that the dma driver is taking responsibility for setting
this correctly.  How would this work otherwise... HBA driver or
subsystem queries the dmaengine device and then sets this parameter on
its behalf?  In other words dmanengine *is* the subsystem, if I am
understanding your definition.

[PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[Dan Williams]

On Tue, Jun 7, 2011 at 10:23 PM, FUJITA Tomonori
<fujita.tomonori@lab.ntt.co.jp> wrote:
> On Mon, 6 Jun 2011 11:48:56 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
>
>> dmaengine expands the class of dma providers to include standalone dma
>> agents on a host bus (or elsewhere) in addition to the traditional bus
>> mastering host-bus-adapters that the existing api understands. ?So in
>> the case of slave dma the dma capabilities of the block-device are
>> irrelevant because another agent will do the transfer on behalf of the
>> block-device driver. ?So the value should be whatever the dma device
>> driver says it is.
>
> The dma parameter restriction could be due to software (HBA drivers,
> or subsystem). The value should be whatever the dma device driver says
> it is in such case?

I'm assuming that the dma driver is taking responsibility for setting
this correctly.  How would this work otherwise... HBA driver or
subsystem queries the dmaengine device and then sets this parameter on
its behalf?  In other words dmanengine *is* the subsystem, if I am
understanding your definition.

Re: [PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[FUJITA Tomonori]

On Tue, 7 Jun 2011 23:56:21 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> > The dma parameter restriction could be due to software (HBA drivers,
> > or subsystem). The value should be whatever the dma device driver says
> > it is in such case?
> 
> I'm assuming that the dma driver is taking responsibility for setting
> this correctly.  How would this work otherwise... HBA driver or
> subsystem queries the dmaengine device and then sets this parameter on
> its behalf?  In other words dmanengine *is* the subsystem, if I am
> understanding your definition.

Oops, I meant that the subsystem is software layer above
dmaengine. For example, SCSI subsystem sets the limit of max number of
sglist entries. That is, it is possible that software layer above
dmaengine could set dma limit, which is smaller than the limit of
dmaengine?

[PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[FUJITA Tomonori]

On Tue, 7 Jun 2011 23:56:21 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> > The dma parameter restriction could be due to software (HBA drivers,
> > or subsystem). The value should be whatever the dma device driver says
> > it is in such case?
> 
> I'm assuming that the dma driver is taking responsibility for setting
> this correctly.  How would this work otherwise... HBA driver or
> subsystem queries the dmaengine device and then sets this parameter on
> its behalf?  In other words dmanengine *is* the subsystem, if I am
> understanding your definition.

Oops, I meant that the subsystem is software layer above
dmaengine. For example, SCSI subsystem sets the limit of max number of
sglist entries. That is, it is possible that software layer above
dmaengine could set dma limit, which is smaller than the limit of
dmaengine?

Re: [PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[Dan Williams]

On Wed, Jun 8, 2011 at 12:10 AM, FUJITA Tomonori
<fujita.tomonori@lab.ntt.co.jp> wrote:
> On Tue, 7 Jun 2011 23:56:21 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
>
>> > The dma parameter restriction could be due to software (HBA drivers,
>> > or subsystem). The value should be whatever the dma device driver says
>> > it is in such case?
>>
>> I'm assuming that the dma driver is taking responsibility for setting
>> this correctly.  How would this work otherwise... HBA driver or
>> subsystem queries the dmaengine device and then sets this parameter on
>> its behalf?  In other words dmanengine *is* the subsystem, if I am
>> understanding your definition.
>
> Oops, I meant that the subsystem is software layer above
> dmaengine. For example, SCSI subsystem sets the limit of max number of
> sglist entries. That is, it is possible that software layer above
> dmaengine could set dma limit, which is smaller than the limit of
> dmaengine?
>

Perhaps, but this sounds like the reverse of what happens today where
scsi device drivers with knowledge of their own hardware will tell the
midlayer/subsystem the restriction.  The change with regard to this
patch is that the scsi device driver (for example) will recognize that
the device it is driving will not be a bus master and will arrange to
allocate a dma channel from dmaengine.  When said scsi driver reports
the dma restrictions to the subsystem it will borrow the parameters
from the dma channel, not the scsi device.  So yes, I still think it
should be whatever the dma channel says.

Although, you've been doing scsi work longer than I, so maybe I'm
overlooking something...?

Are there any cases today where the subsystem imposes tighter
restrictions on the dma geometry than what the device reports?  Even
if that were the case it would be same situation that the scsi device
driver reports maximum parameters, but the subsystem opts for
something tighter.  Whether the maximal parameters come from the scsi
device or the dma channel is moot.

Re: [PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[Dan Williams]

On Wed, Jun 8, 2011 at 12:10 AM, FUJITA Tomonori
<fujita.tomonori@lab.ntt.co.jp> wrote:
> On Tue, 7 Jun 2011 23:56:21 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
>
>> > The dma parameter restriction could be due to software (HBA drivers,
>> > or subsystem). The value should be whatever the dma device driver says
>> > it is in such case?
>>
>> I'm assuming that the dma driver is taking responsibility for setting
>> this correctly.  How would this work otherwise... HBA driver or
>> subsystem queries the dmaengine device and then sets this parameter on
>> its behalf?  In other words dmanengine *is* the subsystem, if I am
>> understanding your definition.
>
> Oops, I meant that the subsystem is software layer above
> dmaengine. For example, SCSI subsystem sets the limit of max number of
> sglist entries. That is, it is possible that software layer above
> dmaengine could set dma limit, which is smaller than the limit of
> dmaengine?
>

Perhaps, but this sounds like the reverse of what happens today where
scsi device drivers with knowledge of their own hardware will tell the
midlayer/subsystem the restriction.  The change with regard to this
patch is that the scsi device driver (for example) will recognize that
the device it is driving will not be a bus master and will arrange to
allocate a dma channel from dmaengine.  When said scsi driver reports
the dma restrictions to the subsystem it will borrow the parameters
from the dma channel, not the scsi device.  So yes, I still think it
should be whatever the dma channel says.

Although, you've been doing scsi work longer than I, so maybe I'm
overlooking something...?

Are there any cases today where the subsystem imposes tighter
restrictions on the dma geometry than what the device reports?  Even
if that were the case it would be same situation that the scsi device
driver reports maximum parameters, but the subsystem opts for
something tighter.  Whether the maximal parameters come from the scsi
device or the dma channel is moot.

[PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[Dan Williams]

On Wed, Jun 8, 2011 at 12:10 AM, FUJITA Tomonori
<fujita.tomonori@lab.ntt.co.jp> wrote:
> On Tue, 7 Jun 2011 23:56:21 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
>
>> > The dma parameter restriction could be due to software (HBA drivers,
>> > or subsystem). The value should be whatever the dma device driver says
>> > it is in such case?
>>
>> I'm assuming that the dma driver is taking responsibility for setting
>> this correctly. ?How would this work otherwise... HBA driver or
>> subsystem queries the dmaengine device and then sets this parameter on
>> its behalf? ?In other words dmanengine *is* the subsystem, if I am
>> understanding your definition.
>
> Oops, I meant that the subsystem is software layer above
> dmaengine. For example, SCSI subsystem sets the limit of max number of
> sglist entries. That is, it is possible that software layer above
> dmaengine could set dma limit, which is smaller than the limit of
> dmaengine?
>

Perhaps, but this sounds like the reverse of what happens today where
scsi device drivers with knowledge of their own hardware will tell the
midlayer/subsystem the restriction.  The change with regard to this
patch is that the scsi device driver (for example) will recognize that
the device it is driving will not be a bus master and will arrange to
allocate a dma channel from dmaengine.  When said scsi driver reports
the dma restrictions to the subsystem it will borrow the parameters
from the dma channel, not the scsi device.  So yes, I still think it
should be whatever the dma channel says.

Although, you've been doing scsi work longer than I, so maybe I'm
overlooking something...?

Are there any cases today where the subsystem imposes tighter
restrictions on the dma geometry than what the device reports?  Even
if that were the case it would be same situation that the scsi device
driver reports maximum parameters, but the subsystem opts for
something tighter.  Whether the maximal parameters come from the scsi
device or the dma channel is moot.

Re: [PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[Russell King - ARM Linux]

On Wed, Jun 08, 2011 at 01:05:57PM -0700, Dan Williams wrote:
> Even if that were the case it would be same situation that the scsi device
> driver reports maximum parameters, but the subsystem opts for
> something tighter.  Whether the maximal parameters come from the scsi
> device or the dma channel is moot.

Except for the small issue that many DMA-engine using devices do not
have _any_ DMA capabilities of their own, which means they don't have
anything to put in their own struct device's DMA parameters.  We can't
go around making up random insane parameters in the hope that they'll
exceed whatever DMA-engine is being used with the device - that's a
hack not a solution.

[PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[Russell King - ARM Linux]

On Wed, Jun 08, 2011 at 01:05:57PM -0700, Dan Williams wrote:
> Even if that were the case it would be same situation that the scsi device
> driver reports maximum parameters, but the subsystem opts for
> something tighter.  Whether the maximal parameters come from the scsi
> device or the dma channel is moot.

Except for the small issue that many DMA-engine using devices do not
have _any_ DMA capabilities of their own, which means they don't have
anything to put in their own struct device's DMA parameters.  We can't
go around making up random insane parameters in the hope that they'll
exceed whatever DMA-engine is being used with the device - that's a
hack not a solution.

Re: [PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[Dan Williams]

On Wed, Jun 8, 2011 at 1:41 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Jun 08, 2011 at 01:05:57PM -0700, Dan Williams wrote:
>> Even if that were the case it would be same situation that the scsi device
>> driver reports maximum parameters, but the subsystem opts for
>> something tighter.  Whether the maximal parameters come from the scsi
>> device or the dma channel is moot.
>
> Except for the small issue that many DMA-engine using devices do not
> have _any_ DMA capabilities of their own, which means they don't have
> anything to put in their own struct device's DMA parameters.  We can't
> go around making up random insane parameters in the hope that they'll
> exceed whatever DMA-engine is being used with the device - that's a
> hack not a solution.

So, I was operating under the assumption that most dmaengine drivers
would ignore this api, it's just useful to the subset of slave-dma
consumers that have apriori knowledge that the best answer for the dma
geometry comes from the channel.  But not sure how we prevent abuse of
this api for cases where a better answer is available from another
source, which I think is what Tomonori-san is worried about.

[PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[Dan Williams]

On Wed, Jun 8, 2011 at 1:41 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Jun 08, 2011 at 01:05:57PM -0700, Dan Williams wrote:
>> Even if that were the case it would be same situation that the scsi device
>> driver reports maximum parameters, but the subsystem opts for
>> something tighter. ?Whether the maximal parameters come from the scsi
>> device or the dma channel is moot.
>
> Except for the small issue that many DMA-engine using devices do not
> have _any_ DMA capabilities of their own, which means they don't have
> anything to put in their own struct device's DMA parameters. ?We can't
> go around making up random insane parameters in the hope that they'll
> exceed whatever DMA-engine is being used with the device - that's a
> hack not a solution.

So, I was operating under the assumption that most dmaengine drivers
would ignore this api, it's just useful to the subset of slave-dma
consumers that have apriori knowledge that the best answer for the dma
geometry comes from the channel.  But not sure how we prevent abuse of
this api for cases where a better answer is available from another
source, which I think is what Tomonori-san is worried about.

Re: [PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[FUJITA Tomonori]

On Wed, 8 Jun 2011 13:05:57 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> Perhaps, but this sounds like the reverse of what happens today where
> scsi device drivers with knowledge of their own hardware will tell the
> midlayer/subsystem the restriction.  The change with regard to this
> patch is that the scsi device driver (for example) will recognize that
> the device it is driving will not be a bus master and will arrange to
> allocate a dma channel from dmaengine.  When said scsi driver reports
> the dma restrictions to the subsystem it will borrow the parameters
> from the dma channel, not the scsi device.  So yes, I still think it
> should be whatever the dma channel says.
> 
> Although, you've been doing scsi work longer than I, so maybe I'm
> overlooking something...?
> 
> Are there any cases today where the subsystem imposes tighter
> restrictions on the dma geometry than what the device reports?  Even

Yeah, there are already lots.

SCSI-mid layer set the max sg entries to 128 on many architectures
(see SCSI_MAX_SG_CHAIN_SEGMENTS).

SCSI HBA drivers stores the max sg entries in scsi_host structure.

SCSI-mid layer sets the tighter one (see __scsi_alloc_queue).

	blk_queue_max_segments(q, min_t(unsigned short, shost->sg_tablesize,
					SCSI_MAX_SG_CHAIN_SEGMENTS));

As you know, modern SCSI HBA can handle more sg entries than 128.


> if that were the case it would be same situation that the scsi device
> driver reports maximum parameters, but the subsystem opts for
> something tighter.  Whether the maximal parameters come from the scsi
> device or the dma channel is moot.

If only you convert all the SCSI HBA drivers to store the limit of sg
entries in struct device_dma_parameters and use the proposed API.

I can't find any description in this patchset about how we will
convert software that already set the limit of max sg entries in a
different way.

I don't think that this patchset needs to convert the SCSI (and other
software layers setting the limit of max sg entries). But I think that
we need a new rule, the data structure, and the API about how and
where everyone sets the dma restrictions and tells them to the upper
software layers.

[PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[FUJITA Tomonori]

On Wed, 8 Jun 2011 13:05:57 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> Perhaps, but this sounds like the reverse of what happens today where
> scsi device drivers with knowledge of their own hardware will tell the
> midlayer/subsystem the restriction.  The change with regard to this
> patch is that the scsi device driver (for example) will recognize that
> the device it is driving will not be a bus master and will arrange to
> allocate a dma channel from dmaengine.  When said scsi driver reports
> the dma restrictions to the subsystem it will borrow the parameters
> from the dma channel, not the scsi device.  So yes, I still think it
> should be whatever the dma channel says.
> 
> Although, you've been doing scsi work longer than I, so maybe I'm
> overlooking something...?
> 
> Are there any cases today where the subsystem imposes tighter
> restrictions on the dma geometry than what the device reports?  Even

Yeah, there are already lots.

SCSI-mid layer set the max sg entries to 128 on many architectures
(see SCSI_MAX_SG_CHAIN_SEGMENTS).

SCSI HBA drivers stores the max sg entries in scsi_host structure.

SCSI-mid layer sets the tighter one (see __scsi_alloc_queue).

	blk_queue_max_segments(q, min_t(unsigned short, shost->sg_tablesize,
					SCSI_MAX_SG_CHAIN_SEGMENTS));

As you know, modern SCSI HBA can handle more sg entries than 128.


> if that were the case it would be same situation that the scsi device
> driver reports maximum parameters, but the subsystem opts for
> something tighter.  Whether the maximal parameters come from the scsi
> device or the dma channel is moot.

If only you convert all the SCSI HBA drivers to store the limit of sg
entries in struct device_dma_parameters and use the proposed API.

I can't find any description in this patchset about how we will
convert software that already set the limit of max sg entries in a
different way.

I don't think that this patchset needs to convert the SCSI (and other
software layers setting the limit of max sg entries). But I think that
we need a new rule, the data structure, and the API about how and
where everyone sets the dma restrictions and tells them to the upper
software layers.

Re: [PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[Shawn Guo]

On Tue, Jun 07, 2011 at 03:35:51PM -0700, Dan Williams wrote:
> On Mon, Jun 6, 2011 at 12:30 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> > Like dma_set(get)_max_seg_size for max_segment_size, the patch adds
> > max_segment_number into device_dma_parameters and creates the
> > corresponding dmaengine API dma_set(get)_max_seg_number for it.
> >
> > Here is the user story that tells the need of the new api.  The
> > mxs-mmc is the mmc host controller for Freescale MXS architecture.
> > There are a pair of  mmc host specific parameters max_seg_size and
> > max_segs that mxs-mmc host driver needs to tell mmc core, so that
> > mmc core can know how big each data segment could be and how many
> > segments could be handled one time in a scatter list by host driver.
> >
> > The mxs-mmc driver is one user of dmaengine mxs-dma, and it will call
> > mxs-dma to transfer data in scatter list.  That is to say mxs-mmc has
> > no idea of what max_seg_size and max_segs should be, because they are
> > all mxs-dma capability parameters, and mxs-mmc needs to query them
> > from mxs-dma.
> >
> > Right now, there is well defined dma api (dma_get_max_seg_size) for
> > mmc to query max_seg_size from dma driver, but the one for max_segs
> > is missing.  That's why mxs-mmc driver has to hard-code it.
> >
> > The mxs-mmc is just one example to demonstrate the need of the new
> > api, and there are other mmc host drivers (mxcmmc on imx-dma is
> > another example) and possibly even other dmaengine users need this
> > new api to know the maximum segments that dma driver can handle per
> > dma call.
> >
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > ---
> > Changes since v1:
> >  * Update commit message to explain why the new api is needed
> >
> >  include/linux/device.h      |    1 +
> >  include/linux/dma-mapping.h |   15 +++++++++++++++
> >  2 files changed, 16 insertions(+), 0 deletions(-)
> >
> > diff --git a/include/linux/device.h b/include/linux/device.h
> > index c66111a..44cb2528 100644
> > --- a/include/linux/device.h
> > +++ b/include/linux/device.h
> > @@ -487,6 +487,7 @@ struct device_dma_parameters {
> >         * sg limitations.
> >         */
> 
> Given the discussion seems like this patch is missing an update to the
> documentation of the struct to clarify the definition of dma provider.
>  I.e. that this info belongs to the device doing the dma, and that is
> is not necessarily the same as the device that is requesting dma
> service.
> 
What about the following?

/*
 * device_dma_parameters is a property of DMA provider, and it belongs to the
 * 'struct device' that actually provides DMA service, typically the drivers
 * under drivers/dma, although in some cases the DMA provider and block device
 * uses DMA service happen to be the same 'struct device'.
 *
 * It's not necessary for every single DMA providers to have this structure,
 * because some DMA providers simply do not have these parameters/limitations.
 * For those do have, the DMA providers should be responsible for setting the
 * parameters up.
 */
struct device_dma_parameters {
        unsigned int max_segment_size;
        unsigned int max_segment_number;
        unsigned long segment_boundary_mask;
};

-- 
Regards,
Shawn

Re: [PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[Shawn Guo]

On Tue, Jun 07, 2011 at 03:35:51PM -0700, Dan Williams wrote:
> On Mon, Jun 6, 2011 at 12:30 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> > Like dma_set(get)_max_seg_size for max_segment_size, the patch adds
> > max_segment_number into device_dma_parameters and creates the
> > corresponding dmaengine API dma_set(get)_max_seg_number for it.
> >
> > Here is the user story that tells the need of the new api.  The
> > mxs-mmc is the mmc host controller for Freescale MXS architecture.
> > There are a pair of  mmc host specific parameters max_seg_size and
> > max_segs that mxs-mmc host driver needs to tell mmc core, so that
> > mmc core can know how big each data segment could be and how many
> > segments could be handled one time in a scatter list by host driver.
> >
> > The mxs-mmc driver is one user of dmaengine mxs-dma, and it will call
> > mxs-dma to transfer data in scatter list.  That is to say mxs-mmc has
> > no idea of what max_seg_size and max_segs should be, because they are
> > all mxs-dma capability parameters, and mxs-mmc needs to query them
> > from mxs-dma.
> >
> > Right now, there is well defined dma api (dma_get_max_seg_size) for
> > mmc to query max_seg_size from dma driver, but the one for max_segs
> > is missing.  That's why mxs-mmc driver has to hard-code it.
> >
> > The mxs-mmc is just one example to demonstrate the need of the new
> > api, and there are other mmc host drivers (mxcmmc on imx-dma is
> > another example) and possibly even other dmaengine users need this
> > new api to know the maximum segments that dma driver can handle per
> > dma call.
> >
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > ---
> > Changes since v1:
> >  * Update commit message to explain why the new api is needed
> >
> >  include/linux/device.h      |    1 +
> >  include/linux/dma-mapping.h |   15 +++++++++++++++
> >  2 files changed, 16 insertions(+), 0 deletions(-)
> >
> > diff --git a/include/linux/device.h b/include/linux/device.h
> > index c66111a..44cb2528 100644
> > --- a/include/linux/device.h
> > +++ b/include/linux/device.h
> > @@ -487,6 +487,7 @@ struct device_dma_parameters {
> >         * sg limitations.
> >         */
> 
> Given the discussion seems like this patch is missing an update to the
> documentation of the struct to clarify the definition of dma provider.
>  I.e. that this info belongs to the device doing the dma, and that is
> is not necessarily the same as the device that is requesting dma
> service.
> 
What about the following?

/*
 * device_dma_parameters is a property of DMA provider, and it belongs to the
 * 'struct device' that actually provides DMA service, typically the drivers
 * under drivers/dma, although in some cases the DMA provider and block device
 * uses DMA service happen to be the same 'struct device'.
 *
 * It's not necessary for every single DMA providers to have this structure,
 * because some DMA providers simply do not have these parameters/limitations.
 * For those do have, the DMA providers should be responsible for setting the
 * parameters up.
 */
struct device_dma_parameters {
        unsigned int max_segment_size;
        unsigned int max_segment_number;
        unsigned long segment_boundary_mask;
};

-- 
Regards,
Shawn

[PATCH v2 1/3] dmaengine: add new dma API for max_segment_number

[Shawn Guo]

On Tue, Jun 07, 2011 at 03:35:51PM -0700, Dan Williams wrote:
> On Mon, Jun 6, 2011 at 12:30 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> > Like dma_set(get)_max_seg_size for max_segment_size, the patch adds
> > max_segment_number into device_dma_parameters and creates the
> > corresponding dmaengine API dma_set(get)_max_seg_number for it.
> >
> > Here is the user story that tells the need of the new api. ?The
> > mxs-mmc is the mmc host controller for Freescale MXS architecture.
> > There are a pair of ?mmc host specific parameters max_seg_size and
> > max_segs that mxs-mmc host driver needs to tell mmc core, so that
> > mmc core can know how big each data segment could be and how many
> > segments could be handled one time in a scatter list by host driver.
> >
> > The mxs-mmc driver is one user of dmaengine mxs-dma, and it will call
> > mxs-dma to transfer data in scatter list. ?That is to say mxs-mmc has
> > no idea of what max_seg_size and max_segs should be, because they are
> > all mxs-dma capability parameters, and mxs-mmc needs to query them
> > from mxs-dma.
> >
> > Right now, there is well defined dma api (dma_get_max_seg_size) for
> > mmc to query max_seg_size from dma driver, but the one for max_segs
> > is missing. ?That's why mxs-mmc driver has to hard-code it.
> >
> > The mxs-mmc is just one example to demonstrate the need of the new
> > api, and there are other mmc host drivers (mxcmmc on imx-dma is
> > another example) and possibly even other dmaengine users need this
> > new api to know the maximum segments that dma driver can handle per
> > dma call.
> >
> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > ---
> > Changes since v1:
> > ?* Update commit message to explain why the new api is needed
> >
> > ?include/linux/device.h ? ? ?| ? ?1 +
> > ?include/linux/dma-mapping.h | ? 15 +++++++++++++++
> > ?2 files changed, 16 insertions(+), 0 deletions(-)
> >
> > diff --git a/include/linux/device.h b/include/linux/device.h
> > index c66111a..44cb2528 100644
> > --- a/include/linux/device.h
> > +++ b/include/linux/device.h
> > @@ -487,6 +487,7 @@ struct device_dma_parameters {
> > ? ? ? ? * sg limitations.
> > ? ? ? ? */
> 
> Given the discussion seems like this patch is missing an update to the
> documentation of the struct to clarify the definition of dma provider.
>  I.e. that this info belongs to the device doing the dma, and that is
> is not necessarily the same as the device that is requesting dma
> service.
> 
What about the following?

/*
 * device_dma_parameters is a property of DMA provider, and it belongs to the
 * 'struct device' that actually provides DMA service, typically the drivers
 * under drivers/dma, although in some cases the DMA provider and block device
 * uses DMA service happen to be the same 'struct device'.
 *
 * It's not necessary for every single DMA providers to have this structure,
 * because some DMA providers simply do not have these parameters/limitations.
 * For those do have, the DMA providers should be responsible for setting the
 * parameters up.
 */
struct device_dma_parameters {
        unsigned int max_segment_size;
        unsigned int max_segment_number;
        unsigned long segment_boundary_mask;
};

-- 
Regards,
Shawn

[PATCH v3] dmaengine: add new dma API for max_segment_number

[Shawn Guo]

Like dma_set(get)_max_seg_size for max_segment_size, the patch adds
max_segment_number into device_dma_parameters and creates the
corresponding dmaengine API dma_set(get)_max_seg_number for it.

Here is the user story that tells the need of the new api.  The
mxs-mmc is the mmc host controller for Freescale MXS architecture.
There are a pair of  mmc host specific parameters max_seg_size and
max_segs that mxs-mmc host driver needs to tell mmc core, so that
mmc core can know how big each data segment could be and how many
segments could be handled one time in a scatter list by host driver.

The mxs-mmc driver is one user of dmaengine mxs-dma, and it will call
mxs-dma to transfer data in scatter list.  That is to say mxs-mmc has
no idea of what max_seg_size and max_segs should be, because they are
all mxs-dma capability parameters, and mxs-mmc needs to query them
from mxs-dma.

Right now, there is well defined dma api (dma_get_max_seg_size) for
mmc to query max_seg_size from dma driver, but the one for max_segs
is missing.  That's why mxs-mmc driver has to hard-code it.

The mxs-mmc is just one example to demonstrate the need of the new
api, and there are other mmc host drivers (mxcmmc on imx-dma is
another example) and possibly even other dmaengine users need this
new api to know the maximum segments that dma driver can handle per
dma call.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 include/linux/device.h      |   16 ++++++++++++----
 include/linux/dma-mapping.h |   15 +++++++++++++++
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/include/linux/device.h b/include/linux/device.h
index c66111a..f1152c5 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -481,12 +481,20 @@ extern int devres_release_group(struct device *dev, void *id);
 extern void *devm_kzalloc(struct device *dev, size_t size, gfp_t gfp);
 extern void devm_kfree(struct device *dev, void *p);
 
+/*
+ * device_dma_parameters is a property of DMA provider, and it belongs to the
+ * 'struct device' that actually provides DMA service, typically the drivers
+ * under drivers/dma, although in some cases the DMA provider and block device
+ * uses DMA service happen to be the same 'struct device'.
+ *
+ * It's not necessary for every single DMA providers to have this structure,
+ * because some DMA providers simply do not have these parameters/limitations.
+ * For those do have, the DMA providers should be responsible for setting the
+ * parameters up.
+ */
 struct device_dma_parameters {
-	/*
-	 * a low level driver may set these to teach IOMMU code about
-	 * sg limitations.
-	 */
 	unsigned int max_segment_size;
+	unsigned int max_segment_number;
 	unsigned long segment_boundary_mask;
 };
 
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index ba8319a..fd314f4 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -131,6 +131,21 @@ static inline unsigned int dma_set_max_seg_size(struct device *dev,
 		return -EIO;
 }
 
+static inline unsigned int dma_get_max_seg_number(struct device *dev)
+{
+	return dev->dma_parms ? dev->dma_parms->max_segment_number : 1;
+}
+
+static inline unsigned int dma_set_max_seg_number(struct device *dev,
+						  unsigned int number)
+{
+	if (dev->dma_parms) {
+		dev->dma_parms->max_segment_number = number;
+		return 0;
+	} else
+		return -EIO;
+}
+
 static inline unsigned long dma_get_seg_boundary(struct device *dev)
 {
 	return dev->dma_parms ?
-- 
1.7.4.1

[PATCH v3] dmaengine: add new dma API for max_segment_number

[Shawn Guo]

Like dma_set(get)_max_seg_size for max_segment_size, the patch adds
max_segment_number into device_dma_parameters and creates the
corresponding dmaengine API dma_set(get)_max_seg_number for it.

Here is the user story that tells the need of the new api.  The
mxs-mmc is the mmc host controller for Freescale MXS architecture.
There are a pair of  mmc host specific parameters max_seg_size and
max_segs that mxs-mmc host driver needs to tell mmc core, so that
mmc core can know how big each data segment could be and how many
segments could be handled one time in a scatter list by host driver.

The mxs-mmc driver is one user of dmaengine mxs-dma, and it will call
mxs-dma to transfer data in scatter list.  That is to say mxs-mmc has
no idea of what max_seg_size and max_segs should be, because they are
all mxs-dma capability parameters, and mxs-mmc needs to query them
from mxs-dma.

Right now, there is well defined dma api (dma_get_max_seg_size) for
mmc to query max_seg_size from dma driver, but the one for max_segs
is missing.  That's why mxs-mmc driver has to hard-code it.

The mxs-mmc is just one example to demonstrate the need of the new
api, and there are other mmc host drivers (mxcmmc on imx-dma is
another example) and possibly even other dmaengine users need this
new api to know the maximum segments that dma driver can handle per
dma call.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 include/linux/device.h      |   16 ++++++++++++----
 include/linux/dma-mapping.h |   15 +++++++++++++++
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/include/linux/device.h b/include/linux/device.h
index c66111a..f1152c5 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -481,12 +481,20 @@ extern int devres_release_group(struct device *dev, void *id);
 extern void *devm_kzalloc(struct device *dev, size_t size, gfp_t gfp);
 extern void devm_kfree(struct device *dev, void *p);
 
+/*
+ * device_dma_parameters is a property of DMA provider, and it belongs to the
+ * 'struct device' that actually provides DMA service, typically the drivers
+ * under drivers/dma, although in some cases the DMA provider and block device
+ * uses DMA service happen to be the same 'struct device'.
+ *
+ * It's not necessary for every single DMA providers to have this structure,
+ * because some DMA providers simply do not have these parameters/limitations.
+ * For those do have, the DMA providers should be responsible for setting the
+ * parameters up.
+ */
 struct device_dma_parameters {
-	/*
-	 * a low level driver may set these to teach IOMMU code about
-	 * sg limitations.
-	 */
 	unsigned int max_segment_size;
+	unsigned int max_segment_number;
 	unsigned long segment_boundary_mask;
 };
 
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index ba8319a..fd314f4 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -131,6 +131,21 @@ static inline unsigned int dma_set_max_seg_size(struct device *dev,
 		return -EIO;
 }
 
+static inline unsigned int dma_get_max_seg_number(struct device *dev)
+{
+	return dev->dma_parms ? dev->dma_parms->max_segment_number : 1;
+}
+
+static inline unsigned int dma_set_max_seg_number(struct device *dev,
+						  unsigned int number)
+{
+	if (dev->dma_parms) {
+		dev->dma_parms->max_segment_number = number;
+		return 0;
+	} else
+		return -EIO;
+}
+
 static inline unsigned long dma_get_seg_boundary(struct device *dev)
 {
 	return dev->dma_parms ?
-- 
1.7.4.1

[PATCH v3] dmaengine: add new dma API for max_segment_number

[Shawn Guo]

Like dma_set(get)_max_seg_size for max_segment_size, the patch adds
max_segment_number into device_dma_parameters and creates the
corresponding dmaengine API dma_set(get)_max_seg_number for it.

Here is the user story that tells the need of the new api.  The
mxs-mmc is the mmc host controller for Freescale MXS architecture.
There are a pair of  mmc host specific parameters max_seg_size and
max_segs that mxs-mmc host driver needs to tell mmc core, so that
mmc core can know how big each data segment could be and how many
segments could be handled one time in a scatter list by host driver.

The mxs-mmc driver is one user of dmaengine mxs-dma, and it will call
mxs-dma to transfer data in scatter list.  That is to say mxs-mmc has
no idea of what max_seg_size and max_segs should be, because they are
all mxs-dma capability parameters, and mxs-mmc needs to query them
from mxs-dma.

Right now, there is well defined dma api (dma_get_max_seg_size) for
mmc to query max_seg_size from dma driver, but the one for max_segs
is missing.  That's why mxs-mmc driver has to hard-code it.

The mxs-mmc is just one example to demonstrate the need of the new
api, and there are other mmc host drivers (mxcmmc on imx-dma is
another example) and possibly even other dmaengine users need this
new api to know the maximum segments that dma driver can handle per
dma call.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 include/linux/device.h      |   16 ++++++++++++----
 include/linux/dma-mapping.h |   15 +++++++++++++++
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/include/linux/device.h b/include/linux/device.h
index c66111a..f1152c5 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -481,12 +481,20 @@ extern int devres_release_group(struct device *dev, void *id);
 extern void *devm_kzalloc(struct device *dev, size_t size, gfp_t gfp);
 extern void devm_kfree(struct device *dev, void *p);
 
+/*
+ * device_dma_parameters is a property of DMA provider, and it belongs to the
+ * 'struct device' that actually provides DMA service, typically the drivers
+ * under drivers/dma, although in some cases the DMA provider and block device
+ * uses DMA service happen to be the same 'struct device'.
+ *
+ * It's not necessary for every single DMA providers to have this structure,
+ * because some DMA providers simply do not have these parameters/limitations.
+ * For those do have, the DMA providers should be responsible for setting the
+ * parameters up.
+ */
 struct device_dma_parameters {
-	/*
-	 * a low level driver may set these to teach IOMMU code about
-	 * sg limitations.
-	 */
 	unsigned int max_segment_size;
+	unsigned int max_segment_number;
 	unsigned long segment_boundary_mask;
 };
 
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index ba8319a..fd314f4 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -131,6 +131,21 @@ static inline unsigned int dma_set_max_seg_size(struct device *dev,
 		return -EIO;
 }
 
+static inline unsigned int dma_get_max_seg_number(struct device *dev)
+{
+	return dev->dma_parms ? dev->dma_parms->max_segment_number : 1;
+}
+
+static inline unsigned int dma_set_max_seg_number(struct device *dev,
+						  unsigned int number)
+{
+	if (dev->dma_parms) {
+		dev->dma_parms->max_segment_number = number;
+		return 0;
+	} else
+		return -EIO;
+}
+
 static inline unsigned long dma_get_seg_boundary(struct device *dev)
 {
 	return dev->dma_parms ?
-- 
1.7.4.1

Re: [PATCH v3] dmaengine: add new dma API for max_segment_number

[FUJITA Tomonori]

On Wed, 15 Jun 2011 20:08:41 +0800
Shawn Guo <shawn.guo@linaro.org> wrote:

> Like dma_set(get)_max_seg_size for max_segment_size, the patch adds
> max_segment_number into device_dma_parameters and creates the
> corresponding dmaengine API dma_set(get)_max_seg_number for it.
> 
> Here is the user story that tells the need of the new api.  The
> mxs-mmc is the mmc host controller for Freescale MXS architecture.
> There are a pair of  mmc host specific parameters max_seg_size and
> max_segs that mxs-mmc host driver needs to tell mmc core, so that
> mmc core can know how big each data segment could be and how many
> segments could be handled one time in a scatter list by host driver.
> 
> The mxs-mmc driver is one user of dmaengine mxs-dma, and it will call
> mxs-dma to transfer data in scatter list.  That is to say mxs-mmc has
> no idea of what max_seg_size and max_segs should be, because they are
> all mxs-dma capability parameters, and mxs-mmc needs to query them
> from mxs-dma.
> 
> Right now, there is well defined dma api (dma_get_max_seg_size) for
> mmc to query max_seg_size from dma driver, but the one for max_segs
> is missing.  That's why mxs-mmc driver has to hard-code it.
> 
> The mxs-mmc is just one example to demonstrate the need of the new
> api, and there are other mmc host drivers (mxcmmc on imx-dma is
> another example) and possibly even other dmaengine users need this
> new api to know the maximum segments that dma driver can handle per
> dma call.

As I wrote earlier, SCSI midlayer sets this parameter in a different
way (storing this in SCSI specific data structure).

If you add a new generic API for this parameter, please make sure that
the API works well for the others that already do the same
differently. That is, please send this to linux-scsi too (and discuss
with SCSI maintainer).

[PATCH v3] dmaengine: add new dma API for max_segment_number

[FUJITA Tomonori]

On Wed, 15 Jun 2011 20:08:41 +0800
Shawn Guo <shawn.guo@linaro.org> wrote:

> Like dma_set(get)_max_seg_size for max_segment_size, the patch adds
> max_segment_number into device_dma_parameters and creates the
> corresponding dmaengine API dma_set(get)_max_seg_number for it.
> 
> Here is the user story that tells the need of the new api.  The
> mxs-mmc is the mmc host controller for Freescale MXS architecture.
> There are a pair of  mmc host specific parameters max_seg_size and
> max_segs that mxs-mmc host driver needs to tell mmc core, so that
> mmc core can know how big each data segment could be and how many
> segments could be handled one time in a scatter list by host driver.
> 
> The mxs-mmc driver is one user of dmaengine mxs-dma, and it will call
> mxs-dma to transfer data in scatter list.  That is to say mxs-mmc has
> no idea of what max_seg_size and max_segs should be, because they are
> all mxs-dma capability parameters, and mxs-mmc needs to query them
> from mxs-dma.
> 
> Right now, there is well defined dma api (dma_get_max_seg_size) for
> mmc to query max_seg_size from dma driver, but the one for max_segs
> is missing.  That's why mxs-mmc driver has to hard-code it.
> 
> The mxs-mmc is just one example to demonstrate the need of the new
> api, and there are other mmc host drivers (mxcmmc on imx-dma is
> another example) and possibly even other dmaengine users need this
> new api to know the maximum segments that dma driver can handle per
> dma call.

As I wrote earlier, SCSI midlayer sets this parameter in a different
way (storing this in SCSI specific data structure).

If you add a new generic API for this parameter, please make sure that
the API works well for the others that already do the same
differently. That is, please send this to linux-scsi too (and discuss
with SCSI maintainer).

Re: [PATCH v3] dmaengine: add new dma API for max_segment_number

[Per Forlin]

On Wed, Jun 15, 2011 at 2:08 PM, Shawn Guo <shawn.guo@linaro.org> wrote:
> [PATCH v3] dmaengine: add new dma API for max_segment_number
The subject "dmaengine:" in this case is a bit misleading since the
patches doesn't touch any code inside drivers/dmaengine.
For this patch I would prefer subject "dma-mapping:"

There is a need for the DMA clients to be able to extract the max seg
number from the DMA provider/device. I think this patch is a sane way
of making this information available.

Regards,
Per

> Like dma_set(get)_max_seg_size for max_segment_size, the patch adds
> max_segment_number into device_dma_parameters and creates the
> corresponding dmaengine API dma_set(get)_max_seg_number for it.
>
> Here is the user story that tells the need of the new api.  The
> mxs-mmc is the mmc host controller for Freescale MXS architecture.
> There are a pair of  mmc host specific parameters max_seg_size and
> max_segs that mxs-mmc host driver needs to tell mmc core, so that
> mmc core can know how big each data segment could be and how many
> segments could be handled one time in a scatter list by host driver.
>
> The mxs-mmc driver is one user of dmaengine mxs-dma, and it will call
> mxs-dma to transfer data in scatter list.  That is to say mxs-mmc has
> no idea of what max_seg_size and max_segs should be, because they are
> all mxs-dma capability parameters, and mxs-mmc needs to query them
> from mxs-dma.
>
> Right now, there is well defined dma api (dma_get_max_seg_size) for
> mmc to query max_seg_size from dma driver, but the one for max_segs
> is missing.  That's why mxs-mmc driver has to hard-code it.
>
> The mxs-mmc is just one example to demonstrate the need of the new
> api, and there are other mmc host drivers (mxcmmc on imx-dma is
> another example) and possibly even other dmaengine users need this
> new api to know the maximum segments that dma driver can handle per
> dma call.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  include/linux/device.h      |   16 ++++++++++++----
>  include/linux/dma-mapping.h |   15 +++++++++++++++
>  2 files changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/device.h b/include/linux/device.h
> index c66111a..f1152c5 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -481,12 +481,20 @@ extern int devres_release_group(struct device *dev, void *id);
>  extern void *devm_kzalloc(struct device *dev, size_t size, gfp_t gfp);
>  extern void devm_kfree(struct device *dev, void *p);
>
> +/*
> + * device_dma_parameters is a property of DMA provider, and it belongs to the
> + * 'struct device' that actually provides DMA service, typically the drivers
> + * under drivers/dma, although in some cases the DMA provider and block device
> + * uses DMA service happen to be the same 'struct device'.
> + *
> + * It's not necessary for every single DMA providers to have this structure,
> + * because some DMA providers simply do not have these parameters/limitations.
> + * For those do have, the DMA providers should be responsible for setting the
> + * parameters up.
> + */
>  struct device_dma_parameters {
> -       /*
> -        * a low level driver may set these to teach IOMMU code about
> -        * sg limitations.
> -        */
>        unsigned int max_segment_size;
> +       unsigned int max_segment_number;
>        unsigned long segment_boundary_mask;
>  };
>
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index ba8319a..fd314f4 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -131,6 +131,21 @@ static inline unsigned int dma_set_max_seg_size(struct device *dev,
>                return -EIO;
>  }
>
> +static inline unsigned int dma_get_max_seg_number(struct device *dev)
> +{
> +       return dev->dma_parms ? dev->dma_parms->max_segment_number : 1;
> +}
> +
> +static inline unsigned int dma_set_max_seg_number(struct device *dev,
> +                                                 unsigned int number)
> +{
> +       if (dev->dma_parms) {
> +               dev->dma_parms->max_segment_number = number;
> +               return 0;
> +       } else
> +               return -EIO;
> +}
> +
>  static inline unsigned long dma_get_seg_boundary(struct device *dev)
>  {
>        return dev->dma_parms ?
> --
> 1.7.4.1
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

Re: [PATCH v3] dmaengine: add new dma API for max_segment_number

[Per Forlin]

On Wed, Jun 15, 2011 at 2:08 PM, Shawn Guo <shawn.guo@linaro.org> wrote:
> [PATCH v3] dmaengine: add new dma API for max_segment_number
The subject "dmaengine:" in this case is a bit misleading since the
patches doesn't touch any code inside drivers/dmaengine.
For this patch I would prefer subject "dma-mapping:"

There is a need for the DMA clients to be able to extract the max seg
number from the DMA provider/device. I think this patch is a sane way
of making this information available.

Regards,
Per

> Like dma_set(get)_max_seg_size for max_segment_size, the patch adds
> max_segment_number into device_dma_parameters and creates the
> corresponding dmaengine API dma_set(get)_max_seg_number for it.
>
> Here is the user story that tells the need of the new api.  The
> mxs-mmc is the mmc host controller for Freescale MXS architecture.
> There are a pair of  mmc host specific parameters max_seg_size and
> max_segs that mxs-mmc host driver needs to tell mmc core, so that
> mmc core can know how big each data segment could be and how many
> segments could be handled one time in a scatter list by host driver.
>
> The mxs-mmc driver is one user of dmaengine mxs-dma, and it will call
> mxs-dma to transfer data in scatter list.  That is to say mxs-mmc has
> no idea of what max_seg_size and max_segs should be, because they are
> all mxs-dma capability parameters, and mxs-mmc needs to query them
> from mxs-dma.
>
> Right now, there is well defined dma api (dma_get_max_seg_size) for
> mmc to query max_seg_size from dma driver, but the one for max_segs
> is missing.  That's why mxs-mmc driver has to hard-code it.
>
> The mxs-mmc is just one example to demonstrate the need of the new
> api, and there are other mmc host drivers (mxcmmc on imx-dma is
> another example) and possibly even other dmaengine users need this
> new api to know the maximum segments that dma driver can handle per
> dma call.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  include/linux/device.h      |   16 ++++++++++++----
>  include/linux/dma-mapping.h |   15 +++++++++++++++
>  2 files changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/device.h b/include/linux/device.h
> index c66111a..f1152c5 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -481,12 +481,20 @@ extern int devres_release_group(struct device *dev, void *id);
>  extern void *devm_kzalloc(struct device *dev, size_t size, gfp_t gfp);
>  extern void devm_kfree(struct device *dev, void *p);
>
> +/*
> + * device_dma_parameters is a property of DMA provider, and it belongs to the
> + * 'struct device' that actually provides DMA service, typically the drivers
> + * under drivers/dma, although in some cases the DMA provider and block device
> + * uses DMA service happen to be the same 'struct device'.
> + *
> + * It's not necessary for every single DMA providers to have this structure,
> + * because some DMA providers simply do not have these parameters/limitations.
> + * For those do have, the DMA providers should be responsible for setting the
> + * parameters up.
> + */
>  struct device_dma_parameters {
> -       /*
> -        * a low level driver may set these to teach IOMMU code about
> -        * sg limitations.
> -        */
>        unsigned int max_segment_size;
> +       unsigned int max_segment_number;
>        unsigned long segment_boundary_mask;
>  };
>
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index ba8319a..fd314f4 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -131,6 +131,21 @@ static inline unsigned int dma_set_max_seg_size(struct device *dev,
>                return -EIO;
>  }
>
> +static inline unsigned int dma_get_max_seg_number(struct device *dev)
> +{
> +       return dev->dma_parms ? dev->dma_parms->max_segment_number : 1;
> +}
> +
> +static inline unsigned int dma_set_max_seg_number(struct device *dev,
> +                                                 unsigned int number)
> +{
> +       if (dev->dma_parms) {
> +               dev->dma_parms->max_segment_number = number;
> +               return 0;
> +       } else
> +               return -EIO;
> +}
> +
>  static inline unsigned long dma_get_seg_boundary(struct device *dev)
>  {
>        return dev->dma_parms ?
> --
> 1.7.4.1
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

[PATCH v3] dmaengine: add new dma API for max_segment_number

[Per Forlin]

On Wed, Jun 15, 2011 at 2:08 PM, Shawn Guo <shawn.guo@linaro.org> wrote:
> [PATCH v3] dmaengine: add new dma API for max_segment_number
The subject "dmaengine:" in this case is a bit misleading since the
patches doesn't touch any code inside drivers/dmaengine.
For this patch I would prefer subject "dma-mapping:"

There is a need for the DMA clients to be able to extract the max seg
number from the DMA provider/device. I think this patch is a sane way
of making this information available.

Regards,
Per

> Like dma_set(get)_max_seg_size for max_segment_size, the patch adds
> max_segment_number into device_dma_parameters and creates the
> corresponding dmaengine API dma_set(get)_max_seg_number for it.
>
> Here is the user story that tells the need of the new api. ?The
> mxs-mmc is the mmc host controller for Freescale MXS architecture.
> There are a pair of ?mmc host specific parameters max_seg_size and
> max_segs that mxs-mmc host driver needs to tell mmc core, so that
> mmc core can know how big each data segment could be and how many
> segments could be handled one time in a scatter list by host driver.
>
> The mxs-mmc driver is one user of dmaengine mxs-dma, and it will call
> mxs-dma to transfer data in scatter list. ?That is to say mxs-mmc has
> no idea of what max_seg_size and max_segs should be, because they are
> all mxs-dma capability parameters, and mxs-mmc needs to query them
> from mxs-dma.
>
> Right now, there is well defined dma api (dma_get_max_seg_size) for
> mmc to query max_seg_size from dma driver, but the one for max_segs
> is missing. ?That's why mxs-mmc driver has to hard-code it.
>
> The mxs-mmc is just one example to demonstrate the need of the new
> api, and there are other mmc host drivers (mxcmmc on imx-dma is
> another example) and possibly even other dmaengine users need this
> new api to know the maximum segments that dma driver can handle per
> dma call.
>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
> ?include/linux/device.h ? ? ?| ? 16 ++++++++++++----
> ?include/linux/dma-mapping.h | ? 15 +++++++++++++++
> ?2 files changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/device.h b/include/linux/device.h
> index c66111a..f1152c5 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -481,12 +481,20 @@ extern int devres_release_group(struct device *dev, void *id);
> ?extern void *devm_kzalloc(struct device *dev, size_t size, gfp_t gfp);
> ?extern void devm_kfree(struct device *dev, void *p);
>
> +/*
> + * device_dma_parameters is a property of DMA provider, and it belongs to the
> + * 'struct device' that actually provides DMA service, typically the drivers
> + * under drivers/dma, although in some cases the DMA provider and block device
> + * uses DMA service happen to be the same 'struct device'.
> + *
> + * It's not necessary for every single DMA providers to have this structure,
> + * because some DMA providers simply do not have these parameters/limitations.
> + * For those do have, the DMA providers should be responsible for setting the
> + * parameters up.
> + */
> ?struct device_dma_parameters {
> - ? ? ? /*
> - ? ? ? ?* a low level driver may set these to teach IOMMU code about
> - ? ? ? ?* sg limitations.
> - ? ? ? ?*/
> ? ? ? ?unsigned int max_segment_size;
> + ? ? ? unsigned int max_segment_number;
> ? ? ? ?unsigned long segment_boundary_mask;
> ?};
>
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index ba8319a..fd314f4 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -131,6 +131,21 @@ static inline unsigned int dma_set_max_seg_size(struct device *dev,
> ? ? ? ? ? ? ? ?return -EIO;
> ?}
>
> +static inline unsigned int dma_get_max_seg_number(struct device *dev)
> +{
> + ? ? ? return dev->dma_parms ? dev->dma_parms->max_segment_number : 1;
> +}
> +
> +static inline unsigned int dma_set_max_seg_number(struct device *dev,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned int number)
> +{
> + ? ? ? if (dev->dma_parms) {
> + ? ? ? ? ? ? ? dev->dma_parms->max_segment_number = number;
> + ? ? ? ? ? ? ? return 0;
> + ? ? ? } else
> + ? ? ? ? ? ? ? return -EIO;
> +}
> +
> ?static inline unsigned long dma_get_seg_boundary(struct device *dev)
> ?{
> ? ? ? ?return dev->dma_parms ?
> --
> 1.7.4.1
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

[PATCH v3 RESEND] dma-mapping: add new API for max_segment_number

[Shawn Guo]

Like dma_set(get)_max_seg_size for max_segment_size, the patch adds
max_segment_number into device_dma_parameters and creates the
corresponding API dma_set(get)_max_seg_number for it.

Here is the user story that tells the need of the new api.  The
mxs-mmc is the mmc host controller for Freescale MXS architecture.
There are a pair of  mmc host specific parameters max_seg_size and
max_segs that mxs-mmc host driver needs to tell mmc core, so that
mmc core can know how big each data segment could be and how many
segments could be handled one time in a scatter list by host driver.

The mxs-mmc driver is one user of dmaengine mxs-dma, and it will call
mxs-dma to transfer data in scatter list.  That is to say mxs-mmc has
no idea of what max_seg_size and max_segs should be, because they are
all mxs-dma capability parameters, and mxs-mmc needs to query them
from mxs-dma.

Right now, there is well defined dma api (dma_get_max_seg_size) for
mmc to query max_seg_size from dma driver, but the one for max_segs
is missing.  That's why mxs-mmc driver has to hard-code it.

The mxs-mmc is just one example to demonstrate the need of the new
api, and there are other mmc host drivers (mxcmmc on imx-dma is
another example) and possibly even other dmaengine users need this
new api to know the maximum segments that dma driver can handle per
dma call.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
Reasons for resend:
 * Cc linux-scsi for comment, per request from FUJITA Tomonori
 * Take suggestion from Per Forlin to change patch subject to
   dma-mapping

 include/linux/device.h      |   16 ++++++++++++----
 include/linux/dma-mapping.h |   15 +++++++++++++++
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/include/linux/device.h b/include/linux/device.h
index c66111a..f1152c5 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -481,12 +481,20 @@ extern int devres_release_group(struct device *dev, void *id);
 extern void *devm_kzalloc(struct device *dev, size_t size, gfp_t gfp);
 extern void devm_kfree(struct device *dev, void *p);
 
+/*
+ * device_dma_parameters is a property of DMA provider, and it belongs to the
+ * 'struct device' that actually provides DMA service, typically the drivers
+ * under drivers/dma, although in some cases the DMA provider and block device
+ * uses DMA service happen to be the same 'struct device'.
+ *
+ * It's not necessary for every single DMA providers to have this structure,
+ * because some DMA providers simply do not have these parameters/limitations.
+ * For those do have, the DMA providers should be responsible for setting the
+ * parameters up.
+ */
 struct device_dma_parameters {
-	/*
-	 * a low level driver may set these to teach IOMMU code about
-	 * sg limitations.
-	 */
 	unsigned int max_segment_size;
+	unsigned int max_segment_number;
 	unsigned long segment_boundary_mask;
 };
 
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index ba8319a..fd314f4 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -131,6 +131,21 @@ static inline unsigned int dma_set_max_seg_size(struct device *dev,
 		return -EIO;
 }
 
+static inline unsigned int dma_get_max_seg_number(struct device *dev)
+{
+	return dev->dma_parms ? dev->dma_parms->max_segment_number : 1;
+}
+
+static inline unsigned int dma_set_max_seg_number(struct device *dev,
+						  unsigned int number)
+{
+	if (dev->dma_parms) {
+		dev->dma_parms->max_segment_number = number;
+		return 0;
+	} else
+		return -EIO;
+}
+
 static inline unsigned long dma_get_seg_boundary(struct device *dev)
 {
 	return dev->dma_parms ?
-- 
1.7.4.1

[PATCH v3 RESEND] dma-mapping: add new API for max_segment_number

[Shawn Guo]

Like dma_set(get)_max_seg_size for max_segment_size, the patch adds
max_segment_number into device_dma_parameters and creates the
corresponding API dma_set(get)_max_seg_number for it.

Here is the user story that tells the need of the new api.  The
mxs-mmc is the mmc host controller for Freescale MXS architecture.
There are a pair of  mmc host specific parameters max_seg_size and
max_segs that mxs-mmc host driver needs to tell mmc core, so that
mmc core can know how big each data segment could be and how many
segments could be handled one time in a scatter list by host driver.

The mxs-mmc driver is one user of dmaengine mxs-dma, and it will call
mxs-dma to transfer data in scatter list.  That is to say mxs-mmc has
no idea of what max_seg_size and max_segs should be, because they are
all mxs-dma capability parameters, and mxs-mmc needs to query them
from mxs-dma.

Right now, there is well defined dma api (dma_get_max_seg_size) for
mmc to query max_seg_size from dma driver, but the one for max_segs
is missing.  That's why mxs-mmc driver has to hard-code it.

The mxs-mmc is just one example to demonstrate the need of the new
api, and there are other mmc host drivers (mxcmmc on imx-dma is
another example) and possibly even other dmaengine users need this
new api to know the maximum segments that dma driver can handle per
dma call.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
Reasons for resend:
 * Cc linux-scsi for comment, per request from FUJITA Tomonori
 * Take suggestion from Per Forlin to change patch subject to
   dma-mapping

 include/linux/device.h      |   16 ++++++++++++----
 include/linux/dma-mapping.h |   15 +++++++++++++++
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/include/linux/device.h b/include/linux/device.h
index c66111a..f1152c5 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -481,12 +481,20 @@ extern int devres_release_group(struct device *dev, void *id);
 extern void *devm_kzalloc(struct device *dev, size_t size, gfp_t gfp);
 extern void devm_kfree(struct device *dev, void *p);
 
+/*
+ * device_dma_parameters is a property of DMA provider, and it belongs to the
+ * 'struct device' that actually provides DMA service, typically the drivers
+ * under drivers/dma, although in some cases the DMA provider and block device
+ * uses DMA service happen to be the same 'struct device'.
+ *
+ * It's not necessary for every single DMA providers to have this structure,
+ * because some DMA providers simply do not have these parameters/limitations.
+ * For those do have, the DMA providers should be responsible for setting the
+ * parameters up.
+ */
 struct device_dma_parameters {
-	/*
-	 * a low level driver may set these to teach IOMMU code about
-	 * sg limitations.
-	 */
 	unsigned int max_segment_size;
+	unsigned int max_segment_number;
 	unsigned long segment_boundary_mask;
 };
 
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index ba8319a..fd314f4 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -131,6 +131,21 @@ static inline unsigned int dma_set_max_seg_size(struct device *dev,
 		return -EIO;
 }
 
+static inline unsigned int dma_get_max_seg_number(struct device *dev)
+{
+	return dev->dma_parms ? dev->dma_parms->max_segment_number : 1;
+}
+
+static inline unsigned int dma_set_max_seg_number(struct device *dev,
+						  unsigned int number)
+{
+	if (dev->dma_parms) {
+		dev->dma_parms->max_segment_number = number;
+		return 0;
+	} else
+		return -EIO;
+}
+
 static inline unsigned long dma_get_seg_boundary(struct device *dev)
 {
 	return dev->dma_parms ?
-- 
1.7.4.1

Re: [PATCH v3 RESEND] dma-mapping: add new API for max_segment_number

[Matthew Wilcox]

On Thu, Jun 16, 2011 at 08:30:53PM +0800, Shawn Guo wrote:
> Here is the user story that tells the need of the new api.  The
> mxs-mmc is the mmc host controller for Freescale MXS architecture.
> There are a pair of  mmc host specific parameters max_seg_size and
> max_segs that mxs-mmc host driver needs to tell mmc core, so that
> mmc core can know how big each data segment could be and how many
> segments could be handled one time in a scatter list by host driver.
> 
> The mxs-mmc driver is one user of dmaengine mxs-dma, and it will call
> mxs-dma to transfer data in scatter list.  That is to say mxs-mmc has
> no idea of what max_seg_size and max_segs should be, because they are
> all mxs-dma capability parameters, and mxs-mmc needs to query them
> from mxs-dma.

This approach would make sense if mxs-mmc were generic, but it's tied to
mxs-dma, so it can just as well call mxs-dma to find out how many segments
it supports.

> Right now, there is well defined dma api (dma_get_max_seg_size) for
> mmc to query max_seg_size from dma driver, but the one for max_segs
> is missing.  That's why mxs-mmc driver has to hard-code it.
> 
> The mxs-mmc is just one example to demonstrate the need of the new
> api, and there are other mmc host drivers (mxcmmc on imx-dma is
> another example) and possibly even other dmaengine users need this
> new api to know the maximum segments that dma driver can handle per
> dma call.

Again, mxcmmc can just call imx-dma directly.

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

[PATCH v3 RESEND] dma-mapping: add new API for max_segment_number

[Matthew Wilcox]

On Thu, Jun 16, 2011 at 08:30:53PM +0800, Shawn Guo wrote:
> Here is the user story that tells the need of the new api.  The
> mxs-mmc is the mmc host controller for Freescale MXS architecture.
> There are a pair of  mmc host specific parameters max_seg_size and
> max_segs that mxs-mmc host driver needs to tell mmc core, so that
> mmc core can know how big each data segment could be and how many
> segments could be handled one time in a scatter list by host driver.
> 
> The mxs-mmc driver is one user of dmaengine mxs-dma, and it will call
> mxs-dma to transfer data in scatter list.  That is to say mxs-mmc has
> no idea of what max_seg_size and max_segs should be, because they are
> all mxs-dma capability parameters, and mxs-mmc needs to query them
> from mxs-dma.

This approach would make sense if mxs-mmc were generic, but it's tied to
mxs-dma, so it can just as well call mxs-dma to find out how many segments
it supports.

> Right now, there is well defined dma api (dma_get_max_seg_size) for
> mmc to query max_seg_size from dma driver, but the one for max_segs
> is missing.  That's why mxs-mmc driver has to hard-code it.
> 
> The mxs-mmc is just one example to demonstrate the need of the new
> api, and there are other mmc host drivers (mxcmmc on imx-dma is
> another example) and possibly even other dmaengine users need this
> new api to know the maximum segments that dma driver can handle per
> dma call.

Again, mxcmmc can just call imx-dma directly.

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH v3 RESEND] dma-mapping: add new API for max_segment_number

[Per Forlin]

On 17 June 2011 14:40, Matthew Wilcox <matthew@wil.cx> wrote:
> On Thu, Jun 16, 2011 at 08:30:53PM +0800, Shawn Guo wrote:
>> Here is the user story that tells the need of the new api.  The
>> mxs-mmc is the mmc host controller for Freescale MXS architecture.
>> There are a pair of  mmc host specific parameters max_seg_size and
>> max_segs that mxs-mmc host driver needs to tell mmc core, so that
>> mmc core can know how big each data segment could be and how many
>> segments could be handled one time in a scatter list by host driver.
>>
>> The mxs-mmc driver is one user of dmaengine mxs-dma, and it will call
>> mxs-dma to transfer data in scatter list.  That is to say mxs-mmc has
>> no idea of what max_seg_size and max_segs should be, because they are
>> all mxs-dma capability parameters, and mxs-mmc needs to query them
>> from mxs-dma.
>
> This approach would make sense if mxs-mmc were generic, but it's tied to
> mxs-dma, so it can just as well call mxs-dma to find out how many segments
> it supports.
>
mxs-dma use the generic dmaengine.h. The patch is not only intended
for mxs-mmc. MMCI and other mmc host drivers need a way to query
max_seg_number.
It would be possible to store this number within drivers/dmaengine but
this would not work for omap_hsmmc that doesn't use drivers/dmaengine

Regards,
Per

[PATCH v3 RESEND] dma-mapping: add new API for max_segment_number

[Per Forlin]

On 17 June 2011 14:40, Matthew Wilcox <matthew@wil.cx> wrote:
> On Thu, Jun 16, 2011 at 08:30:53PM +0800, Shawn Guo wrote:
>> Here is the user story that tells the need of the new api. ?The
>> mxs-mmc is the mmc host controller for Freescale MXS architecture.
>> There are a pair of ?mmc host specific parameters max_seg_size and
>> max_segs that mxs-mmc host driver needs to tell mmc core, so that
>> mmc core can know how big each data segment could be and how many
>> segments could be handled one time in a scatter list by host driver.
>>
>> The mxs-mmc driver is one user of dmaengine mxs-dma, and it will call
>> mxs-dma to transfer data in scatter list. ?That is to say mxs-mmc has
>> no idea of what max_seg_size and max_segs should be, because they are
>> all mxs-dma capability parameters, and mxs-mmc needs to query them
>> from mxs-dma.
>
> This approach would make sense if mxs-mmc were generic, but it's tied to
> mxs-dma, so it can just as well call mxs-dma to find out how many segments
> it supports.
>
mxs-dma use the generic dmaengine.h. The patch is not only intended
for mxs-mmc. MMCI and other mmc host drivers need a way to query
max_seg_number.
It would be possible to store this number within drivers/dmaengine but
this would not work for omap_hsmmc that doesn't use drivers/dmaengine

Regards,
Per

Re: [PATCH v3 RESEND] dma-mapping: add new API for max_segment_number

[FUJITA Tomonori]

On Fri, 17 Jun 2011 06:40:35 -0600
Matthew Wilcox <matthew@wil.cx> wrote:

> On Thu, Jun 16, 2011 at 08:30:53PM +0800, Shawn Guo wrote:
> > Here is the user story that tells the need of the new api.  The
> > mxs-mmc is the mmc host controller for Freescale MXS architecture.
> > There are a pair of  mmc host specific parameters max_seg_size and
> > max_segs that mxs-mmc host driver needs to tell mmc core, so that
> > mmc core can know how big each data segment could be and how many
> > segments could be handled one time in a scatter list by host driver.
> > 
> > The mxs-mmc driver is one user of dmaengine mxs-dma, and it will call
> > mxs-dma to transfer data in scatter list.  That is to say mxs-mmc has
> > no idea of what max_seg_size and max_segs should be, because they are
> > all mxs-dma capability parameters, and mxs-mmc needs to query them
> > from mxs-dma.
> 
> This approach would make sense if mxs-mmc were generic, but it's tied to
> mxs-dma, so it can just as well call mxs-dma to find out how many segments
> it supports.

SCSI HBA drivers stores the max number of sg entries in
host->sg_tablesize (and scsi-ml tells the block layer about the
limit)? So if we have the generic API to handle the value, scsi HBA
drivers (and scsi-ml) could use it too?

[PATCH v3 RESEND] dma-mapping: add new API for max_segment_number

[FUJITA Tomonori]

On Fri, 17 Jun 2011 06:40:35 -0600
Matthew Wilcox <matthew@wil.cx> wrote:

> On Thu, Jun 16, 2011 at 08:30:53PM +0800, Shawn Guo wrote:
> > Here is the user story that tells the need of the new api.  The
> > mxs-mmc is the mmc host controller for Freescale MXS architecture.
> > There are a pair of  mmc host specific parameters max_seg_size and
> > max_segs that mxs-mmc host driver needs to tell mmc core, so that
> > mmc core can know how big each data segment could be and how many
> > segments could be handled one time in a scatter list by host driver.
> > 
> > The mxs-mmc driver is one user of dmaengine mxs-dma, and it will call
> > mxs-dma to transfer data in scatter list.  That is to say mxs-mmc has
> > no idea of what max_seg_size and max_segs should be, because they are
> > all mxs-dma capability parameters, and mxs-mmc needs to query them
> > from mxs-dma.
> 
> This approach would make sense if mxs-mmc were generic, but it's tied to
> mxs-dma, so it can just as well call mxs-dma to find out how many segments
> it supports.

SCSI HBA drivers stores the max number of sg entries in
host->sg_tablesize (and scsi-ml tells the block layer about the
limit)? So if we have the generic API to handle the value, scsi HBA
drivers (and scsi-ml) could use it too?