Re: [PATCH 5/5] Add DMA engine driver for Freescale MPC85xx processors.
Hi, Dan, Does I have followed your new API? :-) [..] +static struct dma_chan *of_find_dma_chan_by_phandle(phandle phandle) +{ + struct device_node *np; + struct dma_chan *chan; + struct fsl_dma_device *fdev; + + np = of_find_node_by_phandle(phandle); + if (!np || !of_device_is_compatible(np-parent, fsl,dma)) + return NULL; + + fdev = dev_get_drvdata(of_find_device_by_node(np-parent)-dev); + + list_for_each_entry(chan, fdev-common.channels, device_node) + if (to_of_device(to_fsl_chan(chan)-chan_dev)-node == np) + return chan; + return NULL; +} +EXPORT_SYMBOL(of_find_dma_chan_by_phandle); This routine implies that there is a piece of code somewhere that wants to select which channels it can use. A similar effect can be achieved by registering a dma_client with the dmaengine interface ('dma_async_client_register'). Then when the client code makes a call to 'dma_async_client_chan_request' it receives a 'dma_event_callback' for each channel in the system. It will also be asynchronously notified of channels entering and leaving the system. The goal is to share a common infrastructure for channel management. It's speacial codes for our processors. Some device need the speacial DMA channel, such as must be DMA channel 0. So I add these codes. Or, is it possible to add a API for the special DMA channel getting? Timur had mentioned that this is for device tree support, but your comment makes me think that this is for client support. This sounds like a case where you can define a dma_client to find a specific channel, something like: http://marc.info/?l=linux-kernelm=118417229619156w=2 + +static int __devinit of_fsl_dma_probe(struct of_device *dev, + const struct of_device_id *match) +{ + int err; + unsigned int irq; + struct fsl_dma_device *fdev; + + fdev = kzalloc(sizeof(struct fsl_dma_device), GFP_KERNEL); + if (!fdev) { + dev_err(dev-dev, No enough memory for 'priv'\n); + err = -ENOMEM; + goto err; + } + fdev-dev = dev-dev; + INIT_LIST_HEAD(fdev-common.channels); + + /* get DMA controller register base */ + err = of_address_to_resource(dev-node, 0, fdev-reg); + if (err) { + dev_err(dev-dev, Can't get %s property 'reg'\n, + dev-node-full_name); + goto err; + } + + dev_info(dev-dev, Probe the Freescale DMA driver for %s + controller at 0x%08x...\n, + match-compatible, fdev-reg.start); + fdev-reg_base = ioremap(fdev-reg.start, fdev-reg.end + - fdev-reg.start + 1); + + dma_cap_set(DMA_MEMCPY, fdev-common.cap_mask); + fdev-common.device_alloc_chan_resources = fsl_dma_alloc_chan_resources; + fdev-common.device_free_chan_resources = fsl_dma_free_chan_resources; + fdev-common.device_prep_dma_memcpy = fsl_dma_prep_memcpy; + fdev-common.device_is_tx_complete = fsl_dma_is_complete; + fdev-common.device_issue_pending = fsl_dma_memcpy_issue_pending; + fdev-common.device_dependency_added = fsl_dma_dependency_added; + fdev-common.dev = dev-dev; + If this driver adds: dma_cap_set(DMA_INTERRUPT, fdev-common.cap_mask); fdev-common.device_prep_dma_interrupt = fsl_dma_prep_interrupt; It will be able to take advantage of interrupt triggered callbacks in async_tx. Without these changes async_tx will poll for the completion of each transaction. The new API have lacking documents :) I'll make some study here. Yes, I will address that... Thanks! - zw Regards, Dan ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
RE: [PATCH 5/5] Add DMA engine driver for Freescale MPC85xx processors.
Hi, --- /dev/null +++ b/drivers/dma/fsldma.c @@ -0,0 +1,995 @@ Thanks for using kernel-doc notation. However, ... +/** + * fsl_dma_alloc_descriptor - Allocate descriptor from channel's DMA pool. Function parameters need to be listed described here. See Documentation/kernel-doc-nano-HOWTO.txt or other source files for examples. (Applies to all documented function interfaces here.) All right, I'll add full descriptions here. :P + * + * Return - The descriptor allocated. NULL for failed. + */ +static struct fsl_desc_sw *fsl_dma_alloc_descriptor( + struct fsl_dma_chan *fsl_chan, + gfp_t flags) +{ ... +} +/** + * fsl_chan_xfer_ld_queue -- Transfer the link descriptors in channel + * ld_queue. The function's short description (unfortunately) must be on only one line. E.g.: * fsl_chan_xfer_ld_queue - Transfer link descriptors in channel ld_queue. How about it's length greater than 80? + */ +static void fsl_chan_xfer_ld_queue(struct fsl_dma_chan *fsl_chan) +{ ... +} diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h new file mode 100644 index 000..05be9ed --- /dev/null +++ b/drivers/dma/fsldma.h @@ -0,0 +1,188 @@ +struct fsl_dma_chan_regs { + __mix32 mr; /* 0x00 - Mode Register */ + __mix32 sr; /* 0x04 - Status Register */ + __mix64 cdar; /* 0x08 - Cureent descriptor address register */ Current I'll fix it. Thanks! - zw ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
RE: [PATCH 5/5] Add DMA engine driver for Freescale MPC85xx processors.
If this is experimental, perhaps you should mark the depends line as such depends on on DMA_ENGINE PPC EXPERIMENTAL I'll add EXPERIMENTAL for MPC83xx only. [...] + +fsl_dma_memcpy_issue_pending(chan); +while (fsl_dma_is_complete(chan, cookie, NULL, NULL) +!= DMA_SUCCESS); Again, is it possible to hang your thread here? [...] I'll add msleep here. Thanks! - zw ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
RE: [PATCH 5/5] Add DMA engine driver for Freescale MPC85xx processors.
Hi, Dan, Does I have followed your new API? :-) --- Greetings, Please copy me on any updates to this driver, drivers/dma, or crypto/async_tx. Ok. Below are a few review comments... Regards, Dan +/** + * fsl_dma_alloc_descriptor - Allocate descriptor from channel's DMA pool. + * + * Return - The descriptor allocated. NULL for failed. + */ +static struct fsl_desc_sw *fsl_dma_alloc_descriptor( + struct fsl_dma_chan *fsl_chan, + gfp_t flags) +{ + dma_addr_t pdesc; + struct fsl_desc_sw *desc_sw; + + desc_sw = dma_pool_alloc(fsl_chan-desc_pool, flags, pdesc); + if (desc_sw) { + memset(desc_sw, 0, sizeof(struct fsl_desc_sw)); + dma_async_tx_descriptor_init(desc_sw-async_tx, + fsl_chan-common); + desc_sw-async_tx.tx_set_src = fsl_dma_set_src; + desc_sw-async_tx.tx_set_dest = fsl_dma_set_dest; + desc_sw-async_tx.tx_submit = fsl_dma_tx_submit; + INIT_LIST_HEAD(desc_sw-async_tx.tx_list); + desc_sw-async_tx.phys = pdesc; + } + + return desc_sw; +} I suggest pre-allocating the descriptors: 1/ It alleviates the need to initialize these async_tx fields which never change The dma pool has already stored the descriptors in it's list, 2/ The GFP_ATOMIC allocation can be removed. Ok. iop-adma gets by with one PAGE_SIZE buffer (128 descriptors). [..] +static irqreturn_t fsl_dma_chan_do_interrupt(int irq, void *data) +{ + struct fsl_dma_chan *fsl_chan = (struct fsl_dma_chan *)data; + dma_addr_t stat; + + stat = get_sr(fsl_chan); + dev_dbg(fsl_chan-device-dev, event: channel %d, stat = 0x%x\n, + fsl_chan-id, stat); + set_sr(fsl_chan, stat); /* Clear the event register */ + + stat = ~(FSL_DMA_SR_CB | FSL_DMA_SR_CH); + if (!stat) + return IRQ_NONE; + + /* If the link descriptor segment transfer finishes, +* we will recycle the used descriptor. +*/ + if (stat FSL_DMA_SR_EOSI) { + dev_dbg(fsl_chan-device-dev, event: End-of-segments INT\n); + dev_dbg(fsl_chan-device-dev, event: clndar 0x%016llx, + nlndar 0x%016llx\n, (u64)get_cdar(fsl_chan), + (u64)get_ndar(fsl_chan)); + stat = ~FSL_DMA_SR_EOSI; + fsl_chan_ld_cleanup(fsl_chan, 1); + } + + /* If it current transfer is the end-of-transfer, +* we should clear the Channel Start bit for +* prepare next transfer. +*/ + if (stat (FSL_DMA_SR_EOLNI | FSL_DMA_SR_EOCDI)) { + dev_dbg(fsl_chan-device-dev, event: End-of-link INT\n); + stat = ~FSL_DMA_SR_EOLNI; + fsl_chan_xfer_ld_queue(fsl_chan); + } + + if (stat) + dev_dbg(fsl_chan-device-dev, event: unhandled sr 0x%02x\n, + stat); + + dev_dbg(fsl_chan-device-dev, event: Exit\n); + tasklet_hi_schedule(dma_tasklet); + return IRQ_HANDLED; +} This driver implements locking and callbacks inconsistently with how it is done in ioatdma and iop-adma. The big changes are that all locks have been upgraded from 'spin_lock_bh' to 'spin_lock_irqsave', and async_tx callbacks can happen in irq context. I would like to keep everything in bottom-half context to lessen the restrictions on what async_tx clients can perform in their callback routines. What are the implications of moving 'fsl_chan_ld_cleanup' to the tasklet and changing the 'tasklet_hi_schedule' to 'tasklet_schedule'? A good suggestion :), I need some investigation here. [..] +static struct dma_chan *of_find_dma_chan_by_phandle(phandle phandle) +{ + struct device_node *np; + struct dma_chan *chan; + struct fsl_dma_device *fdev; + + np = of_find_node_by_phandle(phandle); + if (!np || !of_device_is_compatible(np-parent, fsl,dma)) + return NULL; + + fdev = dev_get_drvdata(of_find_device_by_node(np-parent)-dev); + + list_for_each_entry(chan, fdev-common.channels, device_node) + if (to_of_device(to_fsl_chan(chan)-chan_dev)-node == np) + return chan; + return NULL; +} +EXPORT_SYMBOL(of_find_dma_chan_by_phandle); This routine implies that there is a piece of code somewhere that wants to select which channels it can use. A similar effect can be achieved by registering a dma_client with the dmaengine interface ('dma_async_client_register'). Then when the client code makes a call to
Re: [PATCH 5/5] Add DMA engine driver for Freescale MPC85xx processors.
On Tue, Sep 11, 2007 at 06:10:53PM +0800, Zhang Wei-r63237 wrote: + + fsl_dma_memcpy_issue_pending(chan); + while (fsl_dma_is_complete(chan, cookie, NULL, NULL) + != DMA_SUCCESS); Again, is it possible to hang your thread here? [...] I'll add msleep here. I think what was being sought was a timout, causing the test to return failure. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 5/5] Add DMA engine driver for Freescale MPC85xx processors.
Zhang Wei-r63237 wrote: +/** + * fsl_chan_xfer_ld_queue -- Transfer the link descriptors in channel + * ld_queue. The function's short description (unfortunately) must be on only one line. E.g.: * fsl_chan_xfer_ld_queue - Transfer link descriptors in channel ld_queue. How about it's length greater than 80? If it's close to 80+, you can just leave it like that. If it's much longer, you can use this format: /** * function_name - short description * @parameters: prm description * * Longer * function * description. */ Thanks, ~Randy ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
RE: [PATCH 5/5] Add DMA engine driver for Freescale MPC85xx processors.
From: Scott Wood [mailto:[EMAIL PROTECTED] Sent: Tuesday, September 11, 2007 7:20 AM To: Zhang Wei-r63237 Cc: Nelson, Shannon; [EMAIL PROTECTED]; linuxppc-dev@ozlabs.org; Williams, Dan J; [EMAIL PROTECTED] Subject: Re: [PATCH 5/5] Add DMA engine driver for Freescale MPC85xx processors. On Tue, Sep 11, 2007 at 06:10:53PM +0800, Zhang Wei-r63237 wrote: + + fsl_dma_memcpy_issue_pending(chan); + while (fsl_dma_is_complete(chan, cookie, NULL, NULL) + != DMA_SUCCESS); Again, is it possible to hang your thread here? [...] I'll add msleep here. I think what was being sought was a timout, causing the test to return failure. -Scott Either a timeout to stop the polling, or msleep() followed by a single call to fsl_dma_is_complete(). However, using the msleep() method is likely to be kinder to the rest of the kernel than polling for very long. sln ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 5/5] Add DMA engine driver for Freescale MPC85xx processors.
Dan Williams wrote: This routine implies that there is a piece of code somewhere that wants to select which channels it can use. A similar effect can be achieved by registering a dma_client with the dmaengine interface ('dma_async_client_register'). Then when the client code makes a call to 'dma_async_client_chan_request' it receives a 'dma_event_callback' for each channel in the system. It will also be asynchronously notified of channels entering and leaving the system. The goal is to share a common infrastructure for channel management. Are you familiar with the flat device tree used for PowerPC systems? The piece of code somewhere is the device tree subsystem that parses the device tree, which is compiled from the .dts files in arch/powerpc/boot/dts. The FDT is how PowerPC systems specify hardware configuration. In the case of 85xx, the FDT contains entries for each DMA device (typically 2), and the entries contain sub-entries for each DMA channel as well as the address of the register sets for each channel. -- Timur Tabi Linux Kernel Developer @ Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 5/5] Add DMA engine driver for Freescale MPC85xx processors.
On 9/9/07, Timur Tabi [EMAIL PROTECTED] wrote: Dan Williams wrote: This routine implies that there is a piece of code somewhere that wants to select which channels it can use. A similar effect can be achieved by registering a dma_client with the dmaengine interface ('dma_async_client_register'). Then when the client code makes a call to 'dma_async_client_chan_request' it receives a 'dma_event_callback' for each channel in the system. It will also be asynchronously notified of channels entering and leaving the system. The goal is to share a common infrastructure for channel management. Are you familiar with the flat device tree used for PowerPC systems? The piece of code somewhere is the device tree subsystem that parses the device tree, which is compiled from the .dts files in arch/powerpc/boot/dts. The FDT is how PowerPC systems specify hardware configuration. In the case of 85xx, the FDT contains entries for each DMA device (typically 2), and the entries contain sub-entries for each DMA channel as well as the address of the register sets for each channel. Ahh, ok then this code is replacing what would normally be handled by the PCI bus enumeration code, or the platform device registrations for iop-adma in arch/arm/mach-iop13xx. Sorry for the noise with this comment. -- Timur Tabi Linux Kernel Developer @ Freescale - Thanks, Dan ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH 5/5] Add DMA engine driver for Freescale MPC85xx processors.
The driver implements DMA engine API for Freescale MPC85xx DMA controller, which could be used for MEM--MEM, IO_ADDR--MEM and IO_ADDR--IO_ADDR data transfer. The driver supports the Basic mode of Freescale MPC85xx DMA controller. The MPC85xx processors supported include MPC8540/60, MPC8555, MPC8548, MPC8641 and so on. The support for MPC83xx(MPC8349, MPC8360) is experimental. Signed-off-by: Zhang Wei [EMAIL PROTECTED] Signed-off-by: Ebony Zhu [EMAIL PROTECTED] --- drivers/dma/Kconfig |8 + drivers/dma/Makefile |1 + drivers/dma/fsldma.c | 995 ++ drivers/dma/fsldma.h | 188 ++ 4 files changed, 1192 insertions(+), 0 deletions(-) create mode 100644 drivers/dma/fsldma.c create mode 100644 drivers/dma/fsldma.h diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig index 8f670da..a99e925 100644 --- a/drivers/dma/Kconfig +++ b/drivers/dma/Kconfig @@ -40,4 +40,12 @@ config INTEL_IOP_ADMA ---help--- Enable support for the Intel(R) IOP Series RAID engines. +config FSL_DMA + bool Freescale MPC85xx/MPC83xx DMA support + depends on DMA_ENGINE PPC + ---help--- + Enable support for the Freescale DMA engine. Now, it support + MPC8560/40, MPC8555, MPC8548 and MPC8641 processors. + The MPC8349, MPC8360 support is experimental. + endmenu diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile index b3839b6..50ab26c 100644 --- a/drivers/dma/Makefile +++ b/drivers/dma/Makefile @@ -2,3 +2,4 @@ obj-$(CONFIG_DMA_ENGINE) += dmaengine.o obj-$(CONFIG_NET_DMA) += iovlock.o obj-$(CONFIG_INTEL_IOATDMA) += ioatdma.o obj-$(CONFIG_INTEL_IOP_ADMA) += iop-adma.o +obj-$(CONFIG_FSL_DMA) += fsldma.o diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c new file mode 100644 index 000..9e2d56b --- /dev/null +++ b/drivers/dma/fsldma.c @@ -0,0 +1,995 @@ +/* + * Freescale MPC85xx, MPC83xx DMA Engine support + * + * Copyright (C) 2007 Freescale Semiconductor, Inc. All rights reserved. + * + * Author: + * Zhang Wei [EMAIL PROTECTED], Jul 2007 + * Ebony Zhu [EMAIL PROTECTED], May 2007 + * + * Description: + * DMA engine driver for Freescale MPC8540 DMA controller, which is + * also fit for MPC8560, MPC8555, MPC8548, MPC8641, and etc. + * The support for MPC8349 DMA contorller is also added. But it's + * ONLY experimental for MPC8349 now. + * + * This is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + */ + +#include linux/init.h +#include linux/module.h +#include linux/pci.h +#include linux/interrupt.h +#include linux/dmaengine.h +#include linux/delay.h +#include linux/dma-mapping.h +#include linux/dmapool.h +#include linux/of_platform.h + +#include fsldma.h + +static LIST_HEAD(recy_ln_chain); /* ld chain for recycle */ +static spinlock_t recy_ln_lock = SPIN_LOCK_UNLOCKED; + +static void dma_do_tasklet(unsigned long unused); +static DECLARE_TASKLET(dma_tasklet, dma_do_tasklet, 0); + +#define to_fsl_chan(chan) container_of(chan, struct fsl_dma_chan, common) +#define to_fsl_desc(lh) container_of(lh, struct fsl_desc_sw, node) +#define tx_to_fsl_desc(tx) container_of(tx, struct fsl_desc_sw, async_tx) + +static void dma_init(struct fsl_dma_chan *fsl_chan) +{ + /* Reset the channel */ + MIX_OUT(fsl_chan, fsl_chan-reg_base-mr, 0, 32); + + switch (fsl_chan-feature FSL_DMA_IP_MASK) { + case FSL_DMA_IP_86XX: + /* Set the channel to below modes: +* EIE - Error interrupt enable +* EOSIE - End of segments interrupt enable (basic mode) +* EOLNIE - End of links interrupt enable +*/ + MIX_OUT(fsl_chan, fsl_chan-reg_base-mr, FSL_DMA_MR_EIE + | FSL_DMA_MR_EOLNIE | FSL_DMA_MR_EOSIE, 32); + break; + case FSL_DMA_IP_83XX: + /* Set the channel to below modes: +* EOTIE - End-of-transfer interrupt enable +*/ + MIX_OUT(fsl_chan, fsl_chan-reg_base-mr, FSL_DMA_MR_EOTIE, + 32); + break; + } + +} + +static void set_sr(struct fsl_dma_chan *fsl_chan, dma_addr_t val) +{ + MIX_OUT(fsl_chan, fsl_chan-reg_base-sr, val, 32); +} + +static dma_addr_t get_sr(struct fsl_dma_chan *fsl_chan) +{ + return MIX_IN(fsl_chan, fsl_chan-reg_base-sr, 32); +} + +static void get_desc(struct fsl_dma_chan *fsl_chan, struct fsl_dma_ld_hw *hw, + struct fsl_ld_desc *ld) +{ + BUG_ON(!hw); + + ld-src = MIX_TO_CPU(fsl_chan, hw-src_addr, 64); + ld-dest = MIX_TO_CPU(fsl_chan, hw-dst_addr, 64); + ld-count = MIX_TO_CPU(fsl_chan, hw-count, 32); + ld-next_ld_desc = MIX_TO_CPU(fsl_chan, hw-next_ln_addr, 64); + + switch
Re: [PATCH 5/5] Add DMA engine driver for Freescale MPC85xx processors.
On Fri, 7 Sep 2007 18:54:18 +0800 Zhang Wei wrote: Signed-off-by: Zhang Wei [EMAIL PROTECTED] Signed-off-by: Ebony Zhu [EMAIL PROTECTED] --- drivers/dma/Kconfig |8 + drivers/dma/Makefile |1 + drivers/dma/fsldma.c | 995 ++ drivers/dma/fsldma.h | 188 ++ 4 files changed, 1192 insertions(+), 0 deletions(-) create mode 100644 drivers/dma/fsldma.c create mode 100644 drivers/dma/fsldma.h --- /dev/null +++ b/drivers/dma/fsldma.c @@ -0,0 +1,995 @@ Thanks for using kernel-doc notation. However, ... +/** + * fsl_dma_alloc_descriptor - Allocate descriptor from channel's DMA pool. Function parameters need to be listed described here. See Documentation/kernel-doc-nano-HOWTO.txt or other source files for examples. (Applies to all documented function interfaces here.) + * + * Return - The descriptor allocated. NULL for failed. + */ +static struct fsl_desc_sw *fsl_dma_alloc_descriptor( + struct fsl_dma_chan *fsl_chan, + gfp_t flags) +{ ... +} +/** + * fsl_chan_xfer_ld_queue -- Transfer the link descriptors in channel + * ld_queue. The function's short description (unfortunately) must be on only one line. E.g.: * fsl_chan_xfer_ld_queue - Transfer link descriptors in channel ld_queue. + */ +static void fsl_chan_xfer_ld_queue(struct fsl_dma_chan *fsl_chan) +{ ... +} diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h new file mode 100644 index 000..05be9ed --- /dev/null +++ b/drivers/dma/fsldma.h @@ -0,0 +1,188 @@ +struct fsl_dma_chan_regs { + __mix32 mr; /* 0x00 - Mode Register */ + __mix32 sr; /* 0x04 - Status Register */ + __mix64 cdar; /* 0x08 - Cureent descriptor address register */ Current + __mix64 sar;/* 0x10 - Source Address Register */ + __mix64 dar;/* 0x18 - Destination Address Register */ + __mix32 bcr;/* 0x20 - Byte Count Register */ + __mix64 ndar; /* 0x24 - Next Descriptor Address Register */ +}; --- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code *** ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev