Would it be useful to report error status when it happens?
+
+ /*
+ * When both completion and error of termination bits set at the
+ * same time, we do not take it as an error. IOW, it only becomes
+ * an error we need to handler here in case of ether it's an bus
+ * error or a termination error with no completion.
+ */
+ stat2 = ((stat2 >> 16) & stat2) | /* bus error */
+ (~(stat2 >> 16) & stat2 & ~stat1); /* termination with no completion */
+
+ /* combine error and completion status for checking */
+ stat1 = (stat2 << 16) | stat1;
+ while (stat1) {
+ int channel = fls(stat1) - 1;
+ struct mxs_dma_chan *mxs_chan =
+ &mxs_dma->mxs_chans[channel % 16];
+
+ if (channel >= 16) {
+ dev_dbg(mxs_dma->dev, "%s: error in channel %d\n",
+ __func__, channel - 16);
+ mxs_dma_reset_chan(mxs_chan);
+ mxs_chan->status = DMA_ERROR;
+ } else {
+ if (mxs_chan->flags & MXS_DMA_SG_LOOP)
+ mxs_chan->status = DMA_IN_PROGRESS;
+ else
+ mxs_chan->status = DMA_SUCCESS;
+ }
+
+ stat1 &= ~(1 << channel);
+
+ if (mxs_chan->desc.callback)
+ mxs_chan->desc.callback(mxs_chan->desc.callback_param);
Callbacks are supposed to happen from tasklet context, not irq context.
Don't you have a struct device for this? Without a struct device,
dma_alloc_coherent() can only assume that it must give you something
suitable for the smallest DMA mask in your system. That seems
to be mxs_dma->dev.
+ if (!mxs_chan->ccw) {
+ ret = -ENOMEM;
+ goto err_alloc;
+ }
+
+ memset(mxs_chan->ccw, 0, PAGE_SIZE);
+
+ ret = request_irq(mxs_chan->chan_irq, mxs_dma_int_handler,
+ flags, "mxs-dma", mxs_dma);
+ if (ret)
+ goto err_irq;
+
+ flags = IRQF_SHARED;
This patch adds dma support for Freescale MXS-based SoC i.MX23/28,
including apbh-dma and apbx-dma.
* apbh-dma and apbx-dma are supported in the driver as two instances,
and have to be filtered by dma clients via device id. It becomes
the convention that apbh-dma always gets registered prior to
apbx-dma.
* apbh-dma is different between mx23 and mx28, hardware version
register is used to handle the differences.
* Every the mxs dma channel is statically assigned to client device
by soc design with fixed irq. The irq number is being passed by
alloc_chan function with mxs_dma_data, and client driver has to
filter the correct channel by its channel id.
* mxs-dma supports pio function besides data transfer. The driver
uses dma_data_direction DMA_NONE to identify the pio mode, and
steals sgl and sg_len to get pio words and numbers from clients.
* mxs dmaengine has some very specific features, like sense function
and the special NAND support (nand_lock, nand_wait4ready). These
are too specific to implemented in generic dmaengine driver.
* The parameter "flags" of prep functions is currently being used to
pass wait4end flag from clients.
* The driver refers to imx-sdma and only a single descriptor is
statically assigned to each channel.
Signed-off-by: Shawn Guo <redacted>
---
arch/arm/mach-mxs/include/mach/dma.h | 16 +
drivers/dma/Kconfig | 8 +
drivers/dma/Makefile | 1 +
drivers/dma/mxs-dma.c | 702 ++++++++++++++++++++++++++++++++++
4 files changed, 727 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-mxs/include/mach/dma.h
create mode 100644 drivers/dma/mxs-dma.c
This patch adds dma support for Freescale MXS-based SoC i.MX23/28,
including apbh-dma and apbx-dma.
* apbh-dma and apbx-dma are supported in the driver as two instances,
and have to be filtered by dma clients via device id. It becomes
the convention that apbh-dma always gets registered prior to
apbx-dma.
* apbh-dma is different between mx23 and mx28, hardware version
register is used to handle the differences.
* Every the mxs dma channel is statically assigned to client device
by soc design with fixed irq. The irq number is being passed by
alloc_chan function with mxs_dma_data, and client driver has to
filter the correct channel by its channel id.
* mxs-dma supports pio function besides data transfer. The driver
uses dma_data_direction DMA_NONE to identify the pio mode, and
steals sgl and sg_len to get pio words and numbers from clients.
* mxs dmaengine has some very specific features, like sense function
and the special NAND support (nand_lock, nand_wait4ready). These
are too specific to implemented in generic dmaengine driver.
* The parameter "flags" of prep functions is currently being used to
pass wait4end flag from clients.
* The driver refers to imx-sdma and only a single descriptor is
statically assigned to each channel.
Signed-off-by: Shawn Guo <redacted>
---
arch/arm/mach-mxs/include/mach/dma.h | 16 +
drivers/dma/Kconfig | 8 +
drivers/dma/Makefile | 1 +
drivers/dma/mxs-dma.c | 702 ++++++++++++++++++++++++++++++++++
4 files changed, 727 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-mxs/include/mach/dma.h
create mode 100644 drivers/dma/mxs-dma.c
Given that the DMA device is fully internal to the SoC and always
present, does it make sense to add it dynamically and to leave
registration to the boards?
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Sat, Feb 05, 2011 at 10:08:12AM +0800, Shawn Guo wrote:
quoted hunk
This patch adds dma support for Freescale MXS-based SoC i.MX23/28,
including apbh-dma and apbx-dma.
* apbh-dma and apbx-dma are supported in the driver as two instances,
and have to be filtered by dma clients via device id. It becomes
the convention that apbh-dma always gets registered prior to
apbx-dma.
* apbh-dma is different between mx23 and mx28, hardware version
register is used to handle the differences.
* Every the mxs dma channel is statically assigned to client device
by soc design with fixed irq. The irq number is being passed by
alloc_chan function with mxs_dma_data, and client driver has to
filter the correct channel by its channel id.
* mxs-dma supports pio function besides data transfer. The driver
uses dma_data_direction DMA_NONE to identify the pio mode, and
steals sgl and sg_len to get pio words and numbers from clients.
* mxs dmaengine has some very specific features, like sense function
and the special NAND support (nand_lock, nand_wait4ready). These
are too specific to implemented in generic dmaengine driver.
* The parameter "flags" of prep functions is currently being used to
pass wait4end flag from clients.
* The driver refers to imx-sdma and only a single descriptor is
statically assigned to each channel.
Signed-off-by: Shawn Guo <redacted>
---
arch/arm/mach-mxs/include/mach/dma.h | 16 +
drivers/dma/Kconfig | 8 +
drivers/dma/Makefile | 1 +
drivers/dma/mxs-dma.c | 702 ++++++++++++++++++++++++++++++++++
4 files changed, 727 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-mxs/include/mach/dma.h
create mode 100644 drivers/dma/mxs-dma.c
Does this have a valid usecase? I would just return some error code
here. pio_num and pio_words are unused in the driver and I don't think
a dmaengine driver should have some kind of PIO fallback.
Does this have a valid usecase? I would just return some error code
here. pio_num and pio_words are unused in the driver and I don't think
a dmaengine driver should have some kind of PIO fallback.
DMA drivers must not perform PIO as a fallback - that's the job of
the driver using the DMA engine API. The reason is that it buggers up
the DMA buffer ownership rules to the extent that data loss will occur
on ARMv6 and later CPUs.
Also note that the struct device to be used for mapping buffers with
the DMA engine is the dma_device's struct device, not the peripheral
device using the DMA engine. The DMA engine device is what's
performing the DMA, not the peripheral device.
+static irqreturn_t mxs_dma_int_handler(int irq, void *dev_id)
+{
+ struct mxs_dma_engine *mxs_dma = dev_id;
+ u32 stat1, stat2;
+
+ /* completion status */
+ stat1 = __raw_readl(mxs_dma->base + HW_APBHX_CTRL1);
+ stat1 &= 0xffff;
+ __mxs_clrl(stat1, mxs_dma->base + HW_APBHX_CTRL1);
+
+ /* error status */
+ stat2 = __raw_readl(mxs_dma->base + HW_APBHX_CTRL2);
+ __mxs_clrl(stat2, mxs_dma->base + HW_APBHX_CTRL2);
+
+ /*
+ * When both completion and error of termination bits set at the
+ * same time, we do not take it as an error. IOW, it only becomes
+ * an error we need to handler here in case of ether it's an bus
+ * error or a termination error with no completion.
+ */
+ stat2 = ((stat2 >> 16) & stat2) | /* bus error */
+ (~(stat2 >> 16) & stat2 & ~stat1); /* termination with no completion */
+
+ /* combine error and completion status for checking */
+ stat1 = (stat2 << 16) | stat1;
+ while (stat1) {
+ int channel = fls(stat1) - 1;
+ struct mxs_dma_chan *mxs_chan =
+ &mxs_dma->mxs_chans[channel % 16];
+
+ if (channel >= 16) {
+ dev_dbg(mxs_dma->dev, "%s: error in channel %d\n",
+ __func__, channel - 16);
+ mxs_dma_reset_chan(mxs_chan);
+ mxs_chan->status = DMA_ERROR;
+ } else {
+ if (mxs_chan->flags & MXS_DMA_SG_LOOP)
+ mxs_chan->status = DMA_IN_PROGRESS;
+ else
+ mxs_chan->status = DMA_SUCCESS;
+ }
+
+ stat1 &= ~(1 << channel);
+
+ if (mxs_chan->desc.callback)
+ mxs_chan->desc.callback(mxs_chan->desc.callback_param);
+
+ if (mxs_chan->status == DMA_SUCCESS)
+ mxs_chan->last_completed = mxs_chan->desc.cookie;
+ }
+
+ return IRQ_HANDLED;
+}
+
+static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
+{
+ struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
+ struct mxs_dma_data *data = chan->private;
+ struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
+ static unsigned long flags;
+ int ret;
+
+ if (!data)
+ return -EINVAL;
+
+ mxs_chan->chan_irq = data->chan_irq;
+
+ mxs_chan->ccw = dma_alloc_coherent(NULL, PAGE_SIZE,
+ &mxs_chan->ccw_phys, GFP_KERNEL);
+ if (!mxs_chan->ccw) {
+ ret = -ENOMEM;
+ goto err_alloc;
+ }
+
+ memset(mxs_chan->ccw, 0, PAGE_SIZE);
+
+ ret = request_irq(mxs_chan->chan_irq, mxs_dma_int_handler,
+ flags, "mxs-dma", mxs_dma);
+ if (ret)
+ goto err_irq;
+
+ flags = IRQF_SHARED;
+
Apart from the fact, that this is initilized after use, the use of
IRQF_SHARED is wrong here. Shared interrupt handlers are for
multiple handlers sharing a single interrupt source, not for multiple
interrupt sources sharing the same handler!
A shared handler must return IRQ_NONE, if it detects that the
interrupt was from a source it does not handle.
Lothar Wa?mann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________
This patch adds dma support for Freescale MXS-based SoC i.MX23/28,
including apbh-dma and apbx-dma.
* apbh-dma and apbx-dma are supported in the driver as two instances,
and have to be filtered by dma clients via device id. It becomes
the convention that apbh-dma always gets registered prior to
apbx-dma.
* apbh-dma is different between mx23 and mx28, hardware version
register is used to handle the differences.
* Every the mxs dma channel is statically assigned to client device
by soc design with fixed irq. The irq number is being passed by
alloc_chan function with mxs_dma_data, and client driver has to
filter the correct channel by its channel id.
* mxs-dma supports pio function besides data transfer. The driver
uses dma_data_direction DMA_NONE to identify the pio mode, and
steals sgl and sg_len to get pio words and numbers from clients.
* mxs dmaengine has some very specific features, like sense function
and the special NAND support (nand_lock, nand_wait4ready). These
are too specific to implemented in generic dmaengine driver.
* The parameter "flags" of prep functions is currently being used to
pass wait4end flag from clients.
* The driver refers to imx-sdma and only a single descriptor is
statically assigned to each channel.
Signed-off-by: Shawn Guo <redacted>
---
arch/arm/mach-mxs/include/mach/dma.h | 16 +
drivers/dma/Kconfig | 8 +
drivers/dma/Makefile | 1 +
drivers/dma/mxs-dma.c | 702 ++++++++++++++++++++++++++++++++++
4 files changed, 727 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-mxs/include/mach/dma.h
create mode 100644 drivers/dma/mxs-dma.c
On Sat, Feb 05, 2011 at 10:08:12AM +0800, Shawn Guo wrote:
quoted
This patch adds dma support for Freescale MXS-based SoC i.MX23/28,
including apbh-dma and apbx-dma.
* apbh-dma and apbx-dma are supported in the driver as two instances,
and have to be filtered by dma clients via device id. It becomes
the convention that apbh-dma always gets registered prior to
apbx-dma.
* apbh-dma is different between mx23 and mx28, hardware version
register is used to handle the differences.
* Every the mxs dma channel is statically assigned to client device
by soc design with fixed irq. The irq number is being passed by
alloc_chan function with mxs_dma_data, and client driver has to
filter the correct channel by its channel id.
* mxs-dma supports pio function besides data transfer. The driver
uses dma_data_direction DMA_NONE to identify the pio mode, and
steals sgl and sg_len to get pio words and numbers from clients.
* mxs dmaengine has some very specific features, like sense function
and the special NAND support (nand_lock, nand_wait4ready). These
are too specific to implemented in generic dmaengine driver.
* The parameter "flags" of prep functions is currently being used to
pass wait4end flag from clients.
* The driver refers to imx-sdma and only a single descriptor is
statically assigned to each channel.
Signed-off-by: Shawn Guo <redacted>
---
arch/arm/mach-mxs/include/mach/dma.h | 16 +
drivers/dma/Kconfig | 8 +
drivers/dma/Makefile | 1 +
drivers/dma/mxs-dma.c | 702 ++++++++++++++++++++++++++++++++++
4 files changed, 727 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-mxs/include/mach/dma.h
create mode 100644 drivers/dma/mxs-dma.c
Does this have a valid usecase? I would just return some error code
here. pio_num and pio_words are unused in the driver and I don't think
a dmaengine driver should have some kind of PIO fallback.
Actually 'PIO' is a misnomer here. It's the free scaled way of
implementing a simple feature (chained DMA with mixed transfer modes)
in a complicated and obfuscated way.
What's behinde the 'PIO' transfers is programming controller registers
via DMA along with the actual DMA data transfer. DMA_NONE simply
means, that the DMA transfer does only the register programming but
does not transfer any payload. The 'pio_words' are the values that are
being written to consecutive locations of e.g. the SPI controller
register address space.
The programming is actually done by DMA, in any case.
Lothar Wa?mann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________
From: Russell King - ARM Linux <hidden> Date: 2011-02-08 16:38:13
On Wed, Feb 09, 2011 at 06:56:36AM +0800, Shawn Guo wrote:
Hi Russell,
Thanks for the review and comments.
Hi Sascha,
Some of the comments here also apply on imx-sdma, as mxs-dma closely
followed imx-sdma implementation. It's appreciated if you can give
some responses to the comments.
On Fri, Feb 04, 2011 at 06:17:49PM +0000, Russell King - ARM Linux wrote:
quoted
Callbacks are supposed to happen from tasklet context, not irq context.
Here is callback copied from mxs-mmc driver. I expect other mxs-dma
client driver's callbacks are as simple as this one.
static void mxs_mmc_dma_irq_callback(void *param)
{
struct mxs_mmc_host *host = param;
host->status = __raw_readl(host->base + HW_SSP_STATUS);
complete(&host->done);
}
Is a simple callback also required be in tasklet context anyhow?
It is part of the DMA engine API specification. It may not matter
for stuff which currently exists, but how do you know that in the
future you won't be re-using existing drivers which do other stuff
in callbacks and do assume that they're correctly called as per the
DMA engine API?
If you're not going to implement your driver to the DMA engine API
specification, there's no point implementing something which looks
like a DMA engine API but isn't.
Hi Russell,
Thanks for the review and comments.
Hi Sascha,
Some of the comments here also apply on imx-sdma, as mxs-dma closely
followed imx-sdma implementation. It's appreciated if you can give
some responses to the comments.
On Fri, Feb 04, 2011 at 06:17:49PM +0000, Russell King - ARM Linux wrote:
On Sat, Feb 05, 2011 at 10:08:12AM +0800, Shawn Guo wrote:
quoted
+struct mxs_dma_ccw_bits {
+ unsigned int command:2;
+#define MXS_DMA_NO_XFER 0x00
+#define MXS_DMA_WRITE 0x01
+#define MXS_DMA_READ 0x02
+#define MXS_DMA_SENSE 0x03 /* not implemented */
+ unsigned int chain:1;
+ unsigned int irq:1;
+ unsigned int nand_lock:1; /* not implemented */
+ unsigned int nand_wait4ready:1; /* not implemented */
+ unsigned int dec_sem:1;
+ unsigned int wait4end:1;
+ unsigned int halt_on_terminate:1;
+ unsigned int terminate_flush:1;
+ unsigned int reserved:2;
+ unsigned int pio_num:4;
+ unsigned int xfer_bytes:16;
+#define MAX_XFER_BYTES 0xffff
+};
Bitfields are subject to endianness issues. Are you sure this is a good
idea?
Honestly, I'm still a little bit new to kernel development. Can you
please suggest the correct way to do this? BP/BM macros like general
register access?
Would it be useful to report error status when it happens?
The only useful error info that matters here is the channel which
has error. The dev_dbg below tells that. You expect dev_err?
quoted
+
+ /*
+ * When both completion and error of termination bits set at the
+ * same time, we do not take it as an error. IOW, it only becomes
+ * an error we need to handler here in case of ether it's an bus
+ * error or a termination error with no completion.
+ */
+ stat2 = ((stat2 >> 16) & stat2) | /* bus error */
+ (~(stat2 >> 16) & stat2 & ~stat1); /* termination with no completion */
+
+ /* combine error and completion status for checking */
+ stat1 = (stat2 << 16) | stat1;
+ while (stat1) {
+ int channel = fls(stat1) - 1;
+ struct mxs_dma_chan *mxs_chan =
+ &mxs_dma->mxs_chans[channel % 16];
+
+ if (channel >= 16) {
+ dev_dbg(mxs_dma->dev, "%s: error in channel %d\n",
+ __func__, channel - 16);
Callbacks are supposed to happen from tasklet context, not irq context.
Here is callback copied from mxs-mmc driver. I expect other mxs-dma
client driver's callbacks are as simple as this one.
static void mxs_mmc_dma_irq_callback(void *param)
{
struct mxs_mmc_host *host = param;
host->status = __raw_readl(host->base + HW_SSP_STATUS);
complete(&host->done);
}
Is a simple callback also required be in tasklet context anyhow?
Don't you have a struct device for this? Without a struct device,
dma_alloc_coherent() can only assume that it must give you something
suitable for the smallest DMA mask in your system. That seems
to be mxs_dma->dev.
OK. With your comment in another reply, this becomes
mxs_dma->dma_device.dev then.
quoted
+ if (!mxs_chan->ccw) {
+ ret = -ENOMEM;
+ goto err_alloc;
+ }
+
+ memset(mxs_chan->ccw, 0, PAGE_SIZE);
+
+ ret = request_irq(mxs_chan->chan_irq, mxs_dma_int_handler,
+ flags, "mxs-dma", mxs_dma);
+ if (ret)
+ goto err_irq;
+
+ flags = IRQF_SHARED;
Initialization of flags after use?
I should have flags initialized as 0. But as Lothar pointed out,
I mistakenly used flag IRQF_SHARED. My intention is to share the
same handler for different irq lines.
Hi Lothar,
Thanks for the review and all the catches.
On Mon, Feb 07, 2011 at 08:37:14AM +0100, Lothar Wa?mann wrote:
Hi,
Shawn Guo writes:
quoted
This patch adds dma support for Freescale MXS-based SoC i.MX23/28,
including apbh-dma and apbx-dma.
* apbh-dma and apbx-dma are supported in the driver as two instances,
and have to be filtered by dma clients via device id. It becomes
the convention that apbh-dma always gets registered prior to
apbx-dma.
* apbh-dma is different between mx23 and mx28, hardware version
register is used to handle the differences.
* Every the mxs dma channel is statically assigned to client device
by soc design with fixed irq. The irq number is being passed by
alloc_chan function with mxs_dma_data, and client driver has to
filter the correct channel by its channel id.
* mxs-dma supports pio function besides data transfer. The driver
uses dma_data_direction DMA_NONE to identify the pio mode, and
steals sgl and sg_len to get pio words and numbers from clients.
* mxs dmaengine has some very specific features, like sense function
and the special NAND support (nand_lock, nand_wait4ready). These
are too specific to implemented in generic dmaengine driver.
* The parameter "flags" of prep functions is currently being used to
pass wait4end flag from clients.
* The driver refers to imx-sdma and only a single descriptor is
statically assigned to each channel.
Signed-off-by: Shawn Guo <redacted>
---
arch/arm/mach-mxs/include/mach/dma.h | 16 +
drivers/dma/Kconfig | 8 +
drivers/dma/Makefile | 1 +
drivers/dma/mxs-dma.c | 702 ++++++++++++++++++++++++++++++++++
4 files changed, 727 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-mxs/include/mach/dma.h
create mode 100644 drivers/dma/mxs-dma.c
^^^^^^^^^^^^^^
This can never be reached. IMO:
| int ret = 0;
| switch(cmd) {
| case DMA_TERMINATE_ALL:
| mxs_dma_disable_chan(mxs_chan);
| break;
| case DMA_PAUSE:
| mxs_dma_pause_chan(mxs_chan);
| break;
| case DMA_RESUME:
| mxs_dma_resume_chan(mxs_chan);
| break;
| default:
| ret = -ENOSYS;
| }
|
| return ret;
would be cleaner (and effectively generates the same code).
Hi Sascha,
Thanks for the review.
On Mon, Feb 07, 2011 at 09:25:21AM +0100, Sascha Hauer wrote:
On Sat, Feb 05, 2011 at 10:08:12AM +0800, Shawn Guo wrote:
quoted
This patch adds dma support for Freescale MXS-based SoC i.MX23/28,
including apbh-dma and apbx-dma.
* apbh-dma and apbx-dma are supported in the driver as two instances,
and have to be filtered by dma clients via device id. It becomes
the convention that apbh-dma always gets registered prior to
apbx-dma.
* apbh-dma is different between mx23 and mx28, hardware version
register is used to handle the differences.
* Every the mxs dma channel is statically assigned to client device
by soc design with fixed irq. The irq number is being passed by
alloc_chan function with mxs_dma_data, and client driver has to
filter the correct channel by its channel id.
* mxs-dma supports pio function besides data transfer. The driver
uses dma_data_direction DMA_NONE to identify the pio mode, and
steals sgl and sg_len to get pio words and numbers from clients.
* mxs dmaengine has some very specific features, like sense function
and the special NAND support (nand_lock, nand_wait4ready). These
are too specific to implemented in generic dmaengine driver.
* The parameter "flags" of prep functions is currently being used to
pass wait4end flag from clients.
* The driver refers to imx-sdma and only a single descriptor is
statically assigned to each channel.
Signed-off-by: Shawn Guo <redacted>
---
arch/arm/mach-mxs/include/mach/dma.h | 16 +
drivers/dma/Kconfig | 8 +
drivers/dma/Makefile | 1 +
drivers/dma/mxs-dma.c | 702 ++++++++++++++++++++++++++++++++++
4 files changed, 727 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-mxs/include/mach/dma.h
create mode 100644 drivers/dma/mxs-dma.c
Does this have a valid usecase? I would just return some error code
here. pio_num and pio_words are unused in the driver and I don't think
a dmaengine driver should have some kind of PIO fallback.
If you happen to have a look at mxs-mmc patch set, you could find it.
I think it makes more sense to match against device names (apbh vs.
apbx) in the client's filter function than to rely on exact numbering.
I agree, if there is already a member like dev_name in dma_device.
There is dev_id but no dev_name. Suggestion on how to use device
name for matching?
Regards,
Shawn
+static irqreturn_t mxs_dma_int_handler(int irq, void *dev_id)
+{
+ struct mxs_dma_engine *mxs_dma = dev_id;
+ u32 stat1, stat2;
+
+ /* completion status */
+ stat1 = __raw_readl(mxs_dma->base + HW_APBHX_CTRL1);
+ stat1 &= 0xffff;
+ __mxs_clrl(stat1, mxs_dma->base + HW_APBHX_CTRL1);
+
+ /* error status */
+ stat2 = __raw_readl(mxs_dma->base + HW_APBHX_CTRL2);
+ __mxs_clrl(stat2, mxs_dma->base + HW_APBHX_CTRL2);
+
+ /*
+ * When both completion and error of termination bits set at the
+ * same time, we do not take it as an error. IOW, it only becomes
+ * an error we need to handler here in case of ether it's an bus
+ * error or a termination error with no completion.
+ */
+ stat2 = ((stat2 >> 16) & stat2) | /* bus error */
+ (~(stat2 >> 16) & stat2 & ~stat1); /* termination with no completion */
+
+ /* combine error and completion status for checking */
+ stat1 = (stat2 << 16) | stat1;
+ while (stat1) {
+ int channel = fls(stat1) - 1;
+ struct mxs_dma_chan *mxs_chan =
+ &mxs_dma->mxs_chans[channel % 16];
+
+ if (channel >= 16) {
+ dev_dbg(mxs_dma->dev, "%s: error in channel %d\n",
+ __func__, channel - 16);
+ mxs_dma_reset_chan(mxs_chan);
+ mxs_chan->status = DMA_ERROR;
+ } else {
+ if (mxs_chan->flags & MXS_DMA_SG_LOOP)
+ mxs_chan->status = DMA_IN_PROGRESS;
+ else
+ mxs_chan->status = DMA_SUCCESS;
+ }
+
+ stat1 &= ~(1 << channel);
+
+ if (mxs_chan->desc.callback)
+ mxs_chan->desc.callback(mxs_chan->desc.callback_param);
+
+ if (mxs_chan->status == DMA_SUCCESS)
+ mxs_chan->last_completed = mxs_chan->desc.cookie;
+ }
+
+ return IRQ_HANDLED;
+}
+
+static int mxs_dma_alloc_chan_resources(struct dma_chan *chan)
+{
+ struct mxs_dma_chan *mxs_chan = to_mxs_dma_chan(chan);
+ struct mxs_dma_data *data = chan->private;
+ struct mxs_dma_engine *mxs_dma = mxs_chan->mxs_dma;
+ static unsigned long flags;
+ int ret;
+
+ if (!data)
+ return -EINVAL;
+
+ mxs_chan->chan_irq = data->chan_irq;
+
+ mxs_chan->ccw = dma_alloc_coherent(NULL, PAGE_SIZE,
+ &mxs_chan->ccw_phys, GFP_KERNEL);
+ if (!mxs_chan->ccw) {
+ ret = -ENOMEM;
+ goto err_alloc;
+ }
+
+ memset(mxs_chan->ccw, 0, PAGE_SIZE);
+
+ ret = request_irq(mxs_chan->chan_irq, mxs_dma_int_handler,
+ flags, "mxs-dma", mxs_dma);
+ if (ret)
+ goto err_irq;
+
+ flags = IRQF_SHARED;
+
Apart from the fact, that this is initilized after use, the use of
IRQF_SHARED is wrong here. Shared interrupt handlers are for
multiple handlers sharing a single interrupt source, not for multiple
interrupt sources sharing the same handler!
A shared handler must return IRQ_NONE, if it detects that the
interrupt was from a source it does not handle.
My bad. Thanks for pointing this out.
Regards,
Shawn
On Mon, Feb 07, 2011 at 03:31:04PM +0100, Lothar Wa?mann wrote:
Hi,
Shawn Guo writes:
quoted
This patch adds dma support for Freescale MXS-based SoC i.MX23/28,
including apbh-dma and apbx-dma.
* apbh-dma and apbx-dma are supported in the driver as two instances,
and have to be filtered by dma clients via device id. It becomes
the convention that apbh-dma always gets registered prior to
apbx-dma.
* apbh-dma is different between mx23 and mx28, hardware version
register is used to handle the differences.
* Every the mxs dma channel is statically assigned to client device
by soc design with fixed irq. The irq number is being passed by
alloc_chan function with mxs_dma_data, and client driver has to
filter the correct channel by its channel id.
* mxs-dma supports pio function besides data transfer. The driver
uses dma_data_direction DMA_NONE to identify the pio mode, and
steals sgl and sg_len to get pio words and numbers from clients.
* mxs dmaengine has some very specific features, like sense function
and the special NAND support (nand_lock, nand_wait4ready). These
are too specific to implemented in generic dmaengine driver.
* The parameter "flags" of prep functions is currently being used to
pass wait4end flag from clients.
* The driver refers to imx-sdma and only a single descriptor is
statically assigned to each channel.
Signed-off-by: Shawn Guo <redacted>
---
arch/arm/mach-mxs/include/mach/dma.h | 16 +
drivers/dma/Kconfig | 8 +
drivers/dma/Makefile | 1 +
drivers/dma/mxs-dma.c | 702 ++++++++++++++++++++++++++++++++++
4 files changed, 727 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-mxs/include/mach/dma.h
create mode 100644 drivers/dma/mxs-dma.c
On Tue, Feb 08, 2011 at 03:41:55PM +0100, Lothar Wa?mann wrote:
Hi,
quoted
On Sat, Feb 05, 2011 at 10:08:12AM +0800, Shawn Guo wrote:
quoted
This patch adds dma support for Freescale MXS-based SoC i.MX23/28,
including apbh-dma and apbx-dma.
* apbh-dma and apbx-dma are supported in the driver as two instances,
and have to be filtered by dma clients via device id. It becomes
the convention that apbh-dma always gets registered prior to
apbx-dma.
* apbh-dma is different between mx23 and mx28, hardware version
register is used to handle the differences.
* Every the mxs dma channel is statically assigned to client device
by soc design with fixed irq. The irq number is being passed by
alloc_chan function with mxs_dma_data, and client driver has to
filter the correct channel by its channel id.
* mxs-dma supports pio function besides data transfer. The driver
uses dma_data_direction DMA_NONE to identify the pio mode, and
steals sgl and sg_len to get pio words and numbers from clients.
* mxs dmaengine has some very specific features, like sense function
and the special NAND support (nand_lock, nand_wait4ready). These
are too specific to implemented in generic dmaengine driver.
* The parameter "flags" of prep functions is currently being used to
pass wait4end flag from clients.
* The driver refers to imx-sdma and only a single descriptor is
statically assigned to each channel.
Signed-off-by: Shawn Guo <redacted>
---
arch/arm/mach-mxs/include/mach/dma.h | 16 +
drivers/dma/Kconfig | 8 +
drivers/dma/Makefile | 1 +
drivers/dma/mxs-dma.c | 702 ++++++++++++++++++++++++++++++++++
4 files changed, 727 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-mxs/include/mach/dma.h
create mode 100644 drivers/dma/mxs-dma.c
Does this have a valid usecase? I would just return some error code
here. pio_num and pio_words are unused in the driver and I don't think
a dmaengine driver should have some kind of PIO fallback.
Actually 'PIO' is a misnomer here. It's the free scaled way of
implementing a simple feature (chained DMA with mixed transfer modes)
in a complicated and obfuscated way.
What's behinde the 'PIO' transfers is programming controller registers
via DMA along with the actual DMA data transfer. DMA_NONE simply
means, that the DMA transfer does only the register programming but
does not transfer any payload. The 'pio_words' are the values that are
being written to consecutive locations of e.g. the SPI controller
register address space.
The programming is actually done by DMA, in any case.
I'm waiting for this reply ;)
i.MX23/28 Reference Manual uses word "PIO" for the working mode that
Lothar has explained. It seems that "PIO" in mxs-dma needs some more
documents.
It's true that mxs dma hardware is designed to program peripheral
registers along with data transfer with ccw chain. But it's hard
for generic dmaengine model to implement that. The client device
driver gets the data in scatter-gather list to transfer. It requires
client driver to manipulate the sgl to get pio ccw inserted properly
to get the "along with" implemented. This is not a reasonable
implementation to me.
I still chose to keep the pio mode in the implementation in "single
step" rather than "along with" way. That means client driver has to
issue one dma request to program client device registers, and issue
another one to transfer data. The natural thought is that the pio
support can totally be saved with cpu programming. But looking at
any mxs dma client device in reference manual, you will find it gets
two irq lines, irq_dma and irq_error. For ssp (mmc) example, when
one mmc command is issued and completed without error, you have to
either polling ssp status register or use pio dma and irq_dma
interrupt to know the completion. That's to say I keep the pio
support in single ccw way to help client device driver utilize the
interrupt capability somehow.
Actually, besides the pio mode, mxs dma hardware has some other
supports that are incompatible with dmaengine driver model, like
sense command and some nand specific supports. I simply chose not
implement them.
Regards,
Shawn
Given that the DMA device is fully internal to the SoC and always
present, does it make sense to add it dynamically and to leave
registration to the boards?
OK. Will make it in initcall. Correct me if this is not what you
expect.
Regards,
Shawn
On Tue, Feb 08, 2011 at 03:41:55PM +0100, Lothar Wa?mann wrote:
[...]
quoted
What's behinde the 'PIO' transfers is programming controller registers
via DMA along with the actual DMA data transfer. DMA_NONE simply
means, that the DMA transfer does only the register programming but
does not transfer any payload. The 'pio_words' are the values that are
being written to consecutive locations of e.g. the SPI controller
register address space.
The programming is actually done by DMA, in any case.
I'm waiting for this reply ;)
i.MX23/28 Reference Manual uses word "PIO" for the working mode that
Lothar has explained. It seems that "PIO" in mxs-dma needs some more
documents.
It's true that mxs dma hardware is designed to program peripheral
registers along with data transfer with ccw chain. But it's hard
for generic dmaengine model to implement that. The client device
driver gets the data in scatter-gather list to transfer. It requires
client driver to manipulate the sgl to get pio ccw inserted properly
to get the "along with" implemented. This is not a reasonable
implementation to me.
I still chose to keep the pio mode in the implementation in "single
step" rather than "along with" way. That means client driver has to
issue one dma request to program client device registers, and issue
another one to transfer data. The natural thought is that the pio
which defeats the purpose of the whole thing. If the software has to
issue a single request for DMA to program the registers it could as
well directly write the registers without any DMA. The purpose of the
embedded 'PIO' transfers is, that you can set up a whole chain of
commands and associated data transfers and have that executed
without any further software intervention.
support can totally be saved with cpu programming. But looking at
any mxs dma client device in reference manual, you will find it gets
two irq lines, irq_dma and irq_error. For ssp (mmc) example, when
one mmc command is issued and completed without error, you have to
either polling ssp status register or use pio dma and irq_dma
interrupt to know the completion. That's to say I keep the pio
You could use the SSP IRQ as well.
Lothar Wa?mann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________
Does this have a valid usecase? I would just return some error code
here. pio_num and pio_words are unused in the driver and I don't think
a dmaengine driver should have some kind of PIO fallback.
If you happen to have a look at mxs-mmc patch set, you could find it.
As Russell already pointed out, the (mmc-) driver should handle this.
quoted
quoted
+
+ /*
+ * dmaengine clients have to use dma_device.dev_id to filter
+ * dma device between apbh and apbx, so need to ensure it is
+ * identical to mxs_dma_engine.dev_id.
+ */
+ if (mxs_dma->dma_device.dev_id != mxs_dma->dev_id) {
+ dev_err(&pdev->dev, "dev_id of dma_device %d differs from mxs_dma_engine %d\n",
+ mxs_dma->dma_device.dev_id, mxs_dma->dev_id);
+ goto err_init;
+ }
I think it makes more sense to match against device names (apbh vs.
apbx) in the client's filter function than to rely on exact numbering.
I agree, if there is already a member like dev_name in dma_device.
There is dev_id but no dev_name. Suggestion on how to use device
name for matching?
Have a look at the implementation of imx_dma_is_general_purpose() and
imx_dma_is_ipu().
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Tue, Feb 08, 2011 at 03:41:55PM +0100, Lothar Wa?mann wrote:
Hi,
quoted
On Sat, Feb 05, 2011 at 10:08:12AM +0800, Shawn Guo wrote:
quoted
This patch adds dma support for Freescale MXS-based SoC i.MX23/28,
including apbh-dma and apbx-dma.
* apbh-dma and apbx-dma are supported in the driver as two instances,
and have to be filtered by dma clients via device id. It becomes
the convention that apbh-dma always gets registered prior to
apbx-dma.
* apbh-dma is different between mx23 and mx28, hardware version
register is used to handle the differences.
* Every the mxs dma channel is statically assigned to client device
by soc design with fixed irq. The irq number is being passed by
alloc_chan function with mxs_dma_data, and client driver has to
filter the correct channel by its channel id.
* mxs-dma supports pio function besides data transfer. The driver
uses dma_data_direction DMA_NONE to identify the pio mode, and
steals sgl and sg_len to get pio words and numbers from clients.
* mxs dmaengine has some very specific features, like sense function
and the special NAND support (nand_lock, nand_wait4ready). These
are too specific to implemented in generic dmaengine driver.
* The parameter "flags" of prep functions is currently being used to
pass wait4end flag from clients.
* The driver refers to imx-sdma and only a single descriptor is
statically assigned to each channel.
Signed-off-by: Shawn Guo <redacted>
---
arch/arm/mach-mxs/include/mach/dma.h | 16 +
drivers/dma/Kconfig | 8 +
drivers/dma/Makefile | 1 +
drivers/dma/mxs-dma.c | 702 ++++++++++++++++++++++++++++++++++
4 files changed, 727 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-mxs/include/mach/dma.h
create mode 100644 drivers/dma/mxs-dma.c
Does this have a valid usecase? I would just return some error code
here. pio_num and pio_words are unused in the driver and I don't think
a dmaengine driver should have some kind of PIO fallback.
Actually 'PIO' is a misnomer here. It's the free scaled way of
implementing a simple feature (chained DMA with mixed transfer modes)
in a complicated and obfuscated way.
What's behinde the 'PIO' transfers is programming controller registers
via DMA along with the actual DMA data transfer. DMA_NONE simply
means, that the DMA transfer does only the register programming but
does not transfer any payload. The 'pio_words' are the values that are
being written to consecutive locations of e.g. the SPI controller
register address space.
The programming is actually done by DMA, in any case.
OK, got it now.
The NAND programming example in the reference manual looks...
interesting. So it's possible to program the DMA engine in a way that it
autonomously sends commands to the NAND controller, reads blocks from
NAND, polls for status, make branches depending on status bits...
This looks more like a DMA programming language.
I think it does not make much sense to try to implement these features
in the dmaengine API. What the dmaengine offers is only a special
usecase of what the controller provides. Also these features are so
hardware specific that it won't make sense to extend the dmaengine API
for them.
So what should we do? Not use these features at all? Create a bypass to
the dmaengine API? Make the dmaengine driver a user of another, i.MX28
specific driver which provides the additional features?
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Wed, Feb 09, 2011 at 09:06:16AM +0100, Lothar Wa?mann wrote:
[...]
quoted
You could use the SSP IRQ as well.
No, I could not. The SSP IRQ (e.g. MX28_INT_SSP0_ERROR) is designed
to indicate an error condition than the completion of mmc command.
Ah, ok. That's the 'advantage' of not having a dedicated MMC/SD
controller, but adding some MMC capabilities to the SSP controller.
Lothar Wa?mann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________
On Wed, Feb 09, 2011 at 09:06:16AM +0100, Lothar Wa?mann wrote:
Hi Shawn,
Shawn Guo writes:
quoted
On Tue, Feb 08, 2011 at 03:41:55PM +0100, Lothar Wa?mann wrote:
[...]
quoted
quoted
What's behinde the 'PIO' transfers is programming controller registers
via DMA along with the actual DMA data transfer. DMA_NONE simply
means, that the DMA transfer does only the register programming but
does not transfer any payload. The 'pio_words' are the values that are
being written to consecutive locations of e.g. the SPI controller
register address space.
The programming is actually done by DMA, in any case.
I'm waiting for this reply ;)
i.MX23/28 Reference Manual uses word "PIO" for the working mode that
Lothar has explained. It seems that "PIO" in mxs-dma needs some more
documents.
It's true that mxs dma hardware is designed to program peripheral
registers along with data transfer with ccw chain. But it's hard
for generic dmaengine model to implement that. The client device
driver gets the data in scatter-gather list to transfer. It requires
client driver to manipulate the sgl to get pio ccw inserted properly
to get the "along with" implemented. This is not a reasonable
implementation to me.
I still chose to keep the pio mode in the implementation in "single
step" rather than "along with" way. That means client driver has to
issue one dma request to program client device registers, and issue
another one to transfer data. The natural thought is that the pio
which defeats the purpose of the whole thing. If the software has to
issue a single request for DMA to program the registers it could as
well directly write the registers without any DMA. The purpose of the
embedded 'PIO' transfers is, that you can set up a whole chain of
commands and associated data transfers and have that executed
without any further software intervention.
quoted
support can totally be saved with cpu programming. But looking at
any mxs dma client device in reference manual, you will find it gets
two irq lines, irq_dma and irq_error. For ssp (mmc) example, when
one mmc command is issued and completed without error, you have to
either polling ssp status register or use pio dma and irq_dma
interrupt to know the completion. That's to say I keep the pio
You could use the SSP IRQ as well.
No, I could not. The SSP IRQ (e.g. MX28_INT_SSP0_ERROR) is designed
to indicate an error condition than the completion of mmc command.
Regards,
Shawn
On Wed, Feb 09, 2011 at 10:09:15AM +0100, Sascha Hauer wrote:
On Tue, Feb 08, 2011 at 03:41:55PM +0100, Lothar Wa?mann wrote:
quoted
Hi,
quoted
On Sat, Feb 05, 2011 at 10:08:12AM +0800, Shawn Guo wrote:
quoted
This patch adds dma support for Freescale MXS-based SoC i.MX23/28,
including apbh-dma and apbx-dma.
* apbh-dma and apbx-dma are supported in the driver as two instances,
and have to be filtered by dma clients via device id. It becomes
the convention that apbh-dma always gets registered prior to
apbx-dma.
* apbh-dma is different between mx23 and mx28, hardware version
register is used to handle the differences.
* Every the mxs dma channel is statically assigned to client device
by soc design with fixed irq. The irq number is being passed by
alloc_chan function with mxs_dma_data, and client driver has to
filter the correct channel by its channel id.
* mxs-dma supports pio function besides data transfer. The driver
uses dma_data_direction DMA_NONE to identify the pio mode, and
steals sgl and sg_len to get pio words and numbers from clients.
* mxs dmaengine has some very specific features, like sense function
and the special NAND support (nand_lock, nand_wait4ready). These
are too specific to implemented in generic dmaengine driver.
* The parameter "flags" of prep functions is currently being used to
pass wait4end flag from clients.
* The driver refers to imx-sdma and only a single descriptor is
statically assigned to each channel.
Signed-off-by: Shawn Guo <redacted>
---
arch/arm/mach-mxs/include/mach/dma.h | 16 +
drivers/dma/Kconfig | 8 +
drivers/dma/Makefile | 1 +
drivers/dma/mxs-dma.c | 702 ++++++++++++++++++++++++++++++++++
4 files changed, 727 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-mxs/include/mach/dma.h
create mode 100644 drivers/dma/mxs-dma.c
Does this have a valid usecase? I would just return some error code
here. pio_num and pio_words are unused in the driver and I don't think
a dmaengine driver should have some kind of PIO fallback.
Actually 'PIO' is a misnomer here. It's the free scaled way of
implementing a simple feature (chained DMA with mixed transfer modes)
in a complicated and obfuscated way.
What's behinde the 'PIO' transfers is programming controller registers
via DMA along with the actual DMA data transfer. DMA_NONE simply
means, that the DMA transfer does only the register programming but
does not transfer any payload. The 'pio_words' are the values that are
being written to consecutive locations of e.g. the SPI controller
register address space.
The programming is actually done by DMA, in any case.
OK, got it now.
The NAND programming example in the reference manual looks...
interesting. So it's possible to program the DMA engine in a way that it
autonomously sends commands to the NAND controller, reads blocks from
NAND, polls for status, make branches depending on status bits...
This is what sense command does. I scanned the dma client drivers
implemented in Freescale BSP and found only NAND driver uses it.
All others use pio and data transfer.
This looks more like a DMA programming language.
I think it does not make much sense to try to implement these features
in the dmaengine API. What the dmaengine offers is only a special
usecase of what the controller provides. Also these features are so
hardware specific that it won't make sense to extend the dmaengine API
for them.
So what should we do? Not use these features at all? Create a bypass to
the dmaengine API? Make the dmaengine driver a user of another, i.MX28
specific driver which provides the additional features?
My preference is not use these NAND specific features at all. And
I think NAND driver can also be implemented with pio and data
commands only.
Regards,
Shawn
Does this have a valid usecase? I would just return some error code
here. pio_num and pio_words are unused in the driver and I don't think
a dmaengine driver should have some kind of PIO fallback.
If you happen to have a look at mxs-mmc patch set, you could find it.
As Russell already pointed out, the (mmc-) driver should handle this.
quoted
quoted
quoted
+
+ /*
+ * dmaengine clients have to use dma_device.dev_id to filter
+ * dma device between apbh and apbx, so need to ensure it is
+ * identical to mxs_dma_engine.dev_id.
+ */
+ if (mxs_dma->dma_device.dev_id != mxs_dma->dev_id) {
+ dev_err(&pdev->dev, "dev_id of dma_device %d differs from mxs_dma_engine %d\n",
+ mxs_dma->dma_device.dev_id, mxs_dma->dev_id);
+ goto err_init;
+ }
I think it makes more sense to match against device names (apbh vs.
apbx) in the client's filter function than to rely on exact numbering.
I agree, if there is already a member like dev_name in dma_device.
There is dev_id but no dev_name. Suggestion on how to use device
name for matching?
Have a look at the implementation of imx_dma_is_general_purpose() and
imx_dma_is_ipu().
On Wed, Feb 09, 2011 at 09:06:16AM +0100, Lothar Wa?mann wrote:
Hi Shawn,
Shawn Guo writes:
quoted
On Tue, Feb 08, 2011 at 03:41:55PM +0100, Lothar Wa?mann wrote:
[...]
quoted
quoted
What's behinde the 'PIO' transfers is programming controller registers
via DMA along with the actual DMA data transfer. DMA_NONE simply
means, that the DMA transfer does only the register programming but
does not transfer any payload. The 'pio_words' are the values that are
being written to consecutive locations of e.g. the SPI controller
register address space.
The programming is actually done by DMA, in any case.
I'm waiting for this reply ;)
i.MX23/28 Reference Manual uses word "PIO" for the working mode that
Lothar has explained. It seems that "PIO" in mxs-dma needs some more
documents.
It's true that mxs dma hardware is designed to program peripheral
registers along with data transfer with ccw chain. But it's hard
for generic dmaengine model to implement that. The client device
driver gets the data in scatter-gather list to transfer. It requires
client driver to manipulate the sgl to get pio ccw inserted properly
to get the "along with" implemented. This is not a reasonable
implementation to me.
I still chose to keep the pio mode in the implementation in "single
step" rather than "along with" way. That means client driver has to
issue one dma request to program client device registers, and issue
another one to transfer data. The natural thought is that the pio
which defeats the purpose of the whole thing. If the software has to
issue a single request for DMA to program the registers it could as
well directly write the registers without any DMA. The purpose of the
embedded 'PIO' transfers is, that you can set up a whole chain of
commands and associated data transfers and have that executed
without any further software intervention.
I'm redefining the flags of dma prep function as whether the current
sg list should be appended to the last one in the ccw chain, so that
data transfer can be done along with pio controller register. Please
see mxs-dma v2 coming soon for details.
Regards,
Shawn
quoted
support can totally be saved with cpu programming. But looking at
any mxs dma client device in reference manual, you will find it gets
two irq lines, irq_dma and irq_error. For ssp (mmc) example, when
one mmc command is issued and completed without error, you have to
either polling ssp status register or use pio dma and irq_dma
interrupt to know the completion. That's to say I keep the pio
You could use the SSP IRQ as well.
Lothar Wa?mann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Gesch?ftsf?hrer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________