* [PATCH RFC 0/8] MPC512x DMA slave s/g support, OF DMA lookup @ 2013-07-12 15:26 ` Gerhard Sittig 0 siblings, 0 replies; 45+ messages in thread From: Gerhard Sittig @ 2013-07-12 15:26 UTC (permalink / raw) To: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Alexander Popov Cc: Lars-Peter Clausen, Vinod Koul, Dan Williams blurb is below the stats Alexander Popov (1): powerpc: mpc512x_dma: add support for data transfers between memory and i/o memory Gerhard Sittig (6): dma: mpc512x: fix start condition in execute() dma: mpc512x: support 'terminate all' control request dts: mpc512x: prepare for preprocessor support dma: mpc512x: use symbolic specifiers for DMA channels dma: mpc512x: register for device tree channel lookup HACK mmc: mxcmmc: enable clocks for the MPC512x Lars-Peter Clausen (1): dma: of: Add common xlate function for matching by channel id .../devicetree/bindings/dma/mpc512x-dma.txt | 55 ++++++ arch/powerpc/boot/dts/ac14xx.dts | 2 +- arch/powerpc/boot/dts/include/dt-bindings | 1 + arch/powerpc/boot/dts/mpc5121.dtsi | 7 +- arch/powerpc/boot/dts/mpc5121ads.dts | 2 +- arch/powerpc/boot/dts/pdm360ng.dts | 2 +- drivers/dma/mpc512x_dma.c | 181 +++++++++++++++++++- drivers/dma/of-dma.c | 47 +++++ drivers/mmc/host/mxcmmc.c | 41 +++-- include/dt-bindings/dma/mpc512x-dma.h | 21 +++ include/linux/of_dma.h | 4 + 11 files changed, 336 insertions(+), 27 deletions(-) create mode 100644 Documentation/devicetree/bindings/dma/mpc512x-dma.txt create mode 120000 arch/powerpc/boot/dts/include/dt-bindings create mode 100644 include/dt-bindings/dma/mpc512x-dma.h this series - introduces slave s/g support (that's support for DMA transfers which involve peripherals in contrast to mem-to-mem transfers) - adds device tree based lookup support for DMA channels - combines floating patches and related feedback which already covered several aspects of what the suggested LPB driver needs, to demonstrate how integration might be done Alexander, I'd suggest that you pickup this series and - squash part 2 (start condition) and optionally part 3 (terminate all) into part 1 (your slave s/g introduction) - drop part 8 (SD card demo) as it was already NAKed and will be obsolete with correct COMMON_CLK support, but was useful during test to have one more DMA client on the MPC512x platform - put your SCLPC support on top of the then OF capable DMA driver - please leave the preprocessor support for DTS files separate from the DMA related DTS part, as it will be part of another series as well (clocks) or might get picked up separately for its being useful or even a dependency in other ways (GPIOs, interrupts) Lars, there is a checkpatch warning about a line longer than 80 characters in the common xlate part -- I didn't dare to change your submission, and the part included here is verbatim from your patchwork 2331091 (original submission) and 2555701 (resend), is there a followup which you can provide or point us to? Arnd, this version addresses your concerns in Anatolij's earlier submissions: - the DMA controller really maps peripheral requesters 1:1 to DMA channels, there's no other mapping or flexibility involved - OF registration in the DMA controller's driver now is non-fatal for compatibility with older device trees - the device tree binding is documented (even if it's in line with the generic DMA binding, being explicit is good, I guess) Vinod, would you take these DMA related patches when Alexander's updated version has passed review? That's what I understood from Lars so far. the series is based on v3.10, each step in the series was build-tested, the series' result including the mxcmmc hack was run-tested with the commands below, moving 170MB of data and using DMA during transfers, the numbers translate to 1.6MB/s throughput for the SD card and some 62.5KB per DMA IRQ on average (does this help verify plausibility?) $ find /media/mmcblk0p1/ -type f | xargs time wc ... 532086 3032411 173164032 total real 1m 47.41s user 0m 8.08s sys 0m 1.69s $ grep -i dma /proc/interrupts 65: 2716 IPIC Level mpc512x_dma $ grep -i mmc /proc/interrupts 20: 5489 IPIC Level mxc-mmc -- 1.7.10.4 ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH RFC 0/8] MPC512x DMA slave s/g support, OF DMA lookup @ 2013-07-12 15:26 ` Gerhard Sittig 0 siblings, 0 replies; 45+ messages in thread From: Gerhard Sittig @ 2013-07-12 15:26 UTC (permalink / raw) To: linuxppc-dev, devicetree-discuss, Alexander Popov Cc: Lars-Peter Clausen, Arnd Bergmann, Vinod Koul, Gerhard Sittig, Dan Williams, Anatolij Gustschin blurb is below the stats Alexander Popov (1): powerpc: mpc512x_dma: add support for data transfers between memory and i/o memory Gerhard Sittig (6): dma: mpc512x: fix start condition in execute() dma: mpc512x: support 'terminate all' control request dts: mpc512x: prepare for preprocessor support dma: mpc512x: use symbolic specifiers for DMA channels dma: mpc512x: register for device tree channel lookup HACK mmc: mxcmmc: enable clocks for the MPC512x Lars-Peter Clausen (1): dma: of: Add common xlate function for matching by channel id .../devicetree/bindings/dma/mpc512x-dma.txt | 55 ++++++ arch/powerpc/boot/dts/ac14xx.dts | 2 +- arch/powerpc/boot/dts/include/dt-bindings | 1 + arch/powerpc/boot/dts/mpc5121.dtsi | 7 +- arch/powerpc/boot/dts/mpc5121ads.dts | 2 +- arch/powerpc/boot/dts/pdm360ng.dts | 2 +- drivers/dma/mpc512x_dma.c | 181 +++++++++++++++++++- drivers/dma/of-dma.c | 47 +++++ drivers/mmc/host/mxcmmc.c | 41 +++-- include/dt-bindings/dma/mpc512x-dma.h | 21 +++ include/linux/of_dma.h | 4 + 11 files changed, 336 insertions(+), 27 deletions(-) create mode 100644 Documentation/devicetree/bindings/dma/mpc512x-dma.txt create mode 120000 arch/powerpc/boot/dts/include/dt-bindings create mode 100644 include/dt-bindings/dma/mpc512x-dma.h this series - introduces slave s/g support (that's support for DMA transfers which involve peripherals in contrast to mem-to-mem transfers) - adds device tree based lookup support for DMA channels - combines floating patches and related feedback which already covered several aspects of what the suggested LPB driver needs, to demonstrate how integration might be done Alexander, I'd suggest that you pickup this series and - squash part 2 (start condition) and optionally part 3 (terminate all) into part 1 (your slave s/g introduction) - drop part 8 (SD card demo) as it was already NAKed and will be obsolete with correct COMMON_CLK support, but was useful during test to have one more DMA client on the MPC512x platform - put your SCLPC support on top of the then OF capable DMA driver - please leave the preprocessor support for DTS files separate from the DMA related DTS part, as it will be part of another series as well (clocks) or might get picked up separately for its being useful or even a dependency in other ways (GPIOs, interrupts) Lars, there is a checkpatch warning about a line longer than 80 characters in the common xlate part -- I didn't dare to change your submission, and the part included here is verbatim from your patchwork 2331091 (original submission) and 2555701 (resend), is there a followup which you can provide or point us to? Arnd, this version addresses your concerns in Anatolij's earlier submissions: - the DMA controller really maps peripheral requesters 1:1 to DMA channels, there's no other mapping or flexibility involved - OF registration in the DMA controller's driver now is non-fatal for compatibility with older device trees - the device tree binding is documented (even if it's in line with the generic DMA binding, being explicit is good, I guess) Vinod, would you take these DMA related patches when Alexander's updated version has passed review? That's what I understood from Lars so far. the series is based on v3.10, each step in the series was build-tested, the series' result including the mxcmmc hack was run-tested with the commands below, moving 170MB of data and using DMA during transfers, the numbers translate to 1.6MB/s throughput for the SD card and some 62.5KB per DMA IRQ on average (does this help verify plausibility?) $ find /media/mmcblk0p1/ -type f | xargs time wc ... 532086 3032411 173164032 total real 1m 47.41s user 0m 8.08s sys 0m 1.69s $ grep -i dma /proc/interrupts 65: 2716 IPIC Level mpc512x_dma $ grep -i mmc /proc/interrupts 20: 5489 IPIC Level mxc-mmc -- 1.7.10.4 ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH RFC 1/8] powerpc: mpc512x_dma: add support for data transfers between memory and i/o memory 2013-07-12 15:26 ` Gerhard Sittig (?) @ 2013-07-12 15:26 ` Gerhard Sittig 2013-07-14 10:05 ` Lars-Peter Clausen -1 siblings, 1 reply; 45+ messages in thread From: Gerhard Sittig @ 2013-07-12 15:26 UTC (permalink / raw) To: linuxppc-dev, devicetree-discuss, Alexander Popov Cc: Vinod Koul, Lars-Peter Clausen, Anatolij Gustschin, Arnd Bergmann, Dan Williams From: Alexander Popov <a13xp0p0v88@gmail.com> From: Alexander Popov <a13xp0p0v88@gmail.com> Data transfers between memory and i/o memory require more delicate TCD (Transfer Control Descriptor) configuration and DMA channel service requests via hardware. dma_device.device_control callback function is needed to configure DMA channel to work with i/o memory. Signed-off-by: Alexander Popov <a13xp0p0v88@gmail.com> --- drivers/dma/mpc512x_dma.c | 147 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 140 insertions(+), 7 deletions(-) diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c index 2d95673..f90b717 100644 --- a/drivers/dma/mpc512x_dma.c +++ b/drivers/dma/mpc512x_dma.c @@ -2,6 +2,7 @@ * Copyright (C) Freescale Semicondutor, Inc. 2007, 2008. * Copyright (C) Semihalf 2009 * Copyright (C) Ilya Yanok, Emcraft Systems 2010 + * Copyright (C) Alexander Popov, Promcontroller 2013 * * Written by Piotr Ziecik <kosmo@semihalf.com>. Hardware description * (defines, structures and comments) was taken from MPC5121 DMA driver @@ -28,11 +29,6 @@ * file called COPYING. */ -/* - * This is initial version of MPC5121 DMA driver. Only memory to memory - * transfers are supported (tested using dmatest module). - */ - #include <linux/module.h> #include <linux/dmaengine.h> #include <linux/dma-mapping.h> @@ -190,9 +186,13 @@ struct mpc_dma_chan { struct list_head completed; struct mpc_dma_tcd *tcd; dma_addr_t tcd_paddr; + u32 tcd_nunits; /* Lock for this structure */ spinlock_t lock; + + /* Channel's peripheral fifo address */ + dma_addr_t per_paddr; }; struct mpc_dma { @@ -256,7 +256,9 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan) prev->tcd->dlast_sga = mdesc->tcd_paddr; prev->tcd->e_sg = 1; - mdesc->tcd->start = 1; + /* only start explicitly on MDDRC channel */ + if (cid == 32) + mdesc->tcd->start = 1; prev = mdesc; } @@ -268,7 +270,15 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan) if (first != prev) mdma->tcd[cid].e_sg = 1; - out_8(&mdma->regs->dmassrt, cid); + + switch (cid) { + case 26: + out_8(&mdma->regs->dmaserq, cid); + break; + case 32: + out_8(&mdma->regs->dmassrt, cid); + break; + } } /* Handle interrupt on one half of DMA controller (32 channels) */ @@ -641,6 +651,126 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src, return &mdesc->desc; } +static struct dma_async_tx_descriptor *mpc_dma_prep_slave_sg( + struct dma_chan *chan, struct scatterlist *sgl, + unsigned int sg_len, enum dma_transfer_direction direction, + unsigned long flags, void *context) +{ + struct mpc_dma *mdma = dma_chan_to_mpc_dma(chan); + struct mpc_dma_chan *mchan = dma_chan_to_mpc_dma_chan(chan); + struct mpc_dma_desc *mdesc = NULL; + struct mpc_dma_tcd *tcd; + unsigned long iflags; + struct scatterlist *sg; + size_t len; + int iter, i; + + if (!list_empty(&mchan->active)) + return NULL; + + for_each_sg(sgl, sg, sg_len, i) { + spin_lock_irqsave(&mchan->lock, iflags); + + mdesc = list_first_entry(&mchan->free, struct mpc_dma_desc, + node); + if (!mdesc) { + spin_unlock_irqrestore(&mchan->lock, iflags); + /* try to free completed descriptors */ + mpc_dma_process_completed(mdma); + return NULL; + } + + list_del(&mdesc->node); + + spin_unlock_irqrestore(&mchan->lock, iflags); + + mdesc->error = 0; + tcd = mdesc->tcd; + + /* Prepare Transfer Control Descriptor for this transaction */ + memset(tcd, 0, sizeof(struct mpc_dma_tcd)); + + if (!IS_ALIGNED(sg_dma_address(sg), 4)) + return NULL; + + if (direction == DMA_DEV_TO_MEM) { + tcd->saddr = mchan->per_paddr; + tcd->daddr = sg_dma_address(sg); + tcd->soff = 0; + tcd->doff = 4; + } else if (direction == DMA_MEM_TO_DEV) { + tcd->saddr = sg_dma_address(sg); + tcd->daddr = mchan->per_paddr; + tcd->soff = 4; + tcd->doff = 0; + } else { + return NULL; + } + tcd->ssize = MPC_DMA_TSIZE_4; + tcd->dsize = MPC_DMA_TSIZE_4; + + len = sg_dma_len(sg); + + if (mchan->tcd_nunits) + tcd->nbytes = mchan->tcd_nunits * 4; + else + tcd->nbytes = 64; + + if (!IS_ALIGNED(len, tcd->nbytes)) + return NULL; + + iter = len / tcd->nbytes; + if (iter > ((1 << 15) - 1)) { /* maximum biter */ + return NULL; /* len is too big */ + } else { + /* citer_linkch contains the high bits of iter */ + tcd->biter = iter & 0x1ff; + tcd->biter_linkch = iter >> 9; + tcd->citer = tcd->biter; + tcd->citer_linkch = tcd->biter_linkch; + } + + tcd->e_sg = 0; + tcd->d_req = 1; + + /* Place descriptor in prepared list */ + spin_lock_irqsave(&mchan->lock, iflags); + list_add_tail(&mdesc->node, &mchan->prepared); + spin_unlock_irqrestore(&mchan->lock, iflags); + } + + /* Return the last descriptor */ + return &mdesc->desc; +} + +static int mpc_dma_device_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd, + unsigned long arg) +{ + struct mpc_dma_chan *mchan = dma_chan_to_mpc_dma_chan(chan); + struct dma_slave_config *cfg = (void *)arg; + + switch (cmd) { + case DMA_SLAVE_CONFIG: + if (cfg->src_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES && + cfg->dst_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES) + return -EINVAL; + + if (cfg->direction == DMA_DEV_TO_MEM) { + mchan->per_paddr = cfg->src_addr; + mchan->tcd_nunits = cfg->src_maxburst; + } else { + mchan->per_paddr = cfg->dst_addr; + mchan->tcd_nunits = cfg->dst_maxburst; + } + + return 0; + default: + return -ENOSYS; + } + + return -EINVAL; +} + static int mpc_dma_probe(struct platform_device *op) { struct device_node *dn = op->dev.of_node; @@ -725,9 +855,12 @@ static int mpc_dma_probe(struct platform_device *op) dma->device_issue_pending = mpc_dma_issue_pending; dma->device_tx_status = mpc_dma_tx_status; dma->device_prep_dma_memcpy = mpc_dma_prep_memcpy; + dma->device_prep_slave_sg = mpc_dma_prep_slave_sg; + dma->device_control = mpc_dma_device_control; INIT_LIST_HEAD(&dma->channels); dma_cap_set(DMA_MEMCPY, dma->cap_mask); + dma_cap_set(DMA_SLAVE, dma->cap_mask); for (i = 0; i < dma->chancnt; i++) { mchan = &mdma->channels[i]; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH RFC 1/8] powerpc: mpc512x_dma: add support for data transfers between memory and i/o memory 2013-07-12 15:26 ` [PATCH RFC 1/8] powerpc: mpc512x_dma: add support for data transfers between memory and i/o memory Gerhard Sittig @ 2013-07-14 10:05 ` Lars-Peter Clausen [not found] ` <51E277E5.8050305-Qo5EllUWu/uELgA04lAiVw@public.gmane.org> 0 siblings, 1 reply; 45+ messages in thread From: Lars-Peter Clausen @ 2013-07-14 10:05 UTC (permalink / raw) To: Gerhard Sittig Cc: Arnd Bergmann, Vinod Koul, devicetree-discuss, Alexander Popov, Dan Williams, Anatolij Gustschin, linuxppc-dev On 07/12/2013 05:26 PM, Gerhard Sittig wrote: [...] ] > + if (mchan->tcd_nunits) > + tcd->nbytes = mchan->tcd_nunits * 4; > + else > + tcd->nbytes = 64; Just wondering where does the magic 64 come from? ^ permalink raw reply [flat|nested] 45+ messages in thread
[parent not found: <51E277E5.8050305-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>]
* Re: [PATCH RFC 1/8] powerpc: mpc512x_dma: add support for data transfers between memory and i/o memory 2013-07-14 10:05 ` Lars-Peter Clausen @ 2013-07-14 11:07 ` Gerhard Sittig 0 siblings, 0 replies; 45+ messages in thread From: Gerhard Sittig @ 2013-07-14 11:07 UTC (permalink / raw) To: Lars-Peter Clausen Cc: Vinod Koul, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Alexander Popov, Dan Williams, linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ On Sun, Jul 14, 2013 at 12:05 +0200, Lars-Peter Clausen wrote: > > On 07/12/2013 05:26 PM, Gerhard Sittig wrote: > [...] > ] > > + if (mchan->tcd_nunits) > > + tcd->nbytes = mchan->tcd_nunits * 4; > > + else > > + tcd->nbytes = 64; > > Just wondering where does the magic 64 come from? "Inherited" from Alexander's submission. By coincidence the SDHC client happened to work with this constraint, as did Alexander's LPB test suite. I'm not aware of other DMA clients as of now (except for the memory transfer, which runs a different prep routine). It looks like there is more potential for generalization and auto adapting to user input (adjusting configurable inner-loop parameters to exactly fullfill the overall job from the user's spec yet meet the chunking constraints of the hardware). virtually yours Gerhard Sittig -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office-ynQEQJNshbs@public.gmane.org ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH RFC 1/8] powerpc: mpc512x_dma: add support for data transfers between memory and i/o memory @ 2013-07-14 11:07 ` Gerhard Sittig 0 siblings, 0 replies; 45+ messages in thread From: Gerhard Sittig @ 2013-07-14 11:07 UTC (permalink / raw) To: Lars-Peter Clausen Cc: Arnd Bergmann, Vinod Koul, devicetree-discuss, Alexander Popov, Dan Williams, Anatolij Gustschin, linuxppc-dev On Sun, Jul 14, 2013 at 12:05 +0200, Lars-Peter Clausen wrote: > > On 07/12/2013 05:26 PM, Gerhard Sittig wrote: > [...] > ] > > + if (mchan->tcd_nunits) > > + tcd->nbytes = mchan->tcd_nunits * 4; > > + else > > + tcd->nbytes = 64; > > Just wondering where does the magic 64 come from? "Inherited" from Alexander's submission. By coincidence the SDHC client happened to work with this constraint, as did Alexander's LPB test suite. I'm not aware of other DMA clients as of now (except for the memory transfer, which runs a different prep routine). It looks like there is more potential for generalization and auto adapting to user input (adjusting configurable inner-loop parameters to exactly fullfill the overall job from the user's spec yet meet the chunking constraints of the hardware). virtually yours Gerhard Sittig -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de ^ permalink raw reply [flat|nested] 45+ messages in thread
[parent not found: <1373642781-32631-1-git-send-email-gsi-ynQEQJNshbs@public.gmane.org>]
* [PATCH RFC 2/8] dma: mpc512x: fix start condition in execute() 2013-07-12 15:26 ` Gerhard Sittig @ 2013-07-12 15:26 ` Gerhard Sittig -1 siblings, 0 replies; 45+ messages in thread From: Gerhard Sittig @ 2013-07-12 15:26 UTC (permalink / raw) To: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Alexander Popov Cc: Lars-Peter Clausen, Vinod Koul, Dan Williams adjust the conditions how submitted DMA jobs get started: memory transfers need to get initiated by an explicit software request, all transfers which involve peripherals need to reference the external requester line Signed-off-by: Gerhard Sittig <gsi-ynQEQJNshbs@public.gmane.org> --- drivers/dma/mpc512x_dma.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c index f90b717..df10a48 100644 --- a/drivers/dma/mpc512x_dma.c +++ b/drivers/dma/mpc512x_dma.c @@ -272,10 +272,12 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan) mdma->tcd[cid].e_sg = 1; switch (cid) { - case 26: + default: + /* peripherals involved, use external request */ out_8(&mdma->regs->dmaserq, cid); break; case 32: + /* memory transfer, software provided start signal */ out_8(&mdma->regs->dmassrt, cid); break; } -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH RFC 2/8] dma: mpc512x: fix start condition in execute() @ 2013-07-12 15:26 ` Gerhard Sittig 0 siblings, 0 replies; 45+ messages in thread From: Gerhard Sittig @ 2013-07-12 15:26 UTC (permalink / raw) To: linuxppc-dev, devicetree-discuss, Alexander Popov Cc: Lars-Peter Clausen, Arnd Bergmann, Vinod Koul, Gerhard Sittig, Dan Williams, Anatolij Gustschin adjust the conditions how submitted DMA jobs get started: memory transfers need to get initiated by an explicit software request, all transfers which involve peripherals need to reference the external requester line Signed-off-by: Gerhard Sittig <gsi@denx.de> --- drivers/dma/mpc512x_dma.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c index f90b717..df10a48 100644 --- a/drivers/dma/mpc512x_dma.c +++ b/drivers/dma/mpc512x_dma.c @@ -272,10 +272,12 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan) mdma->tcd[cid].e_sg = 1; switch (cid) { - case 26: + default: + /* peripherals involved, use external request */ out_8(&mdma->regs->dmaserq, cid); break; case 32: + /* memory transfer, software provided start signal */ out_8(&mdma->regs->dmassrt, cid); break; } -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH RFC 3/8] dma: mpc512x: support 'terminate all' control request 2013-07-12 15:26 ` Gerhard Sittig @ 2013-07-12 15:26 ` Gerhard Sittig -1 siblings, 0 replies; 45+ messages in thread From: Gerhard Sittig @ 2013-07-12 15:26 UTC (permalink / raw) To: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Alexander Popov Cc: Lars-Peter Clausen, Vinod Koul, Dan Williams implement the TERMINATE_ALL request in the device_control() callback of the DMA engine driver for the MPC512x DMA controller reword variable initialization to better follow the code path and to avoid artificial diffs later on (this style change vanishes when this patch gets squashed with the device_control() routine's introduction) Signed-off-by: Gerhard Sittig <gsi-ynQEQJNshbs@public.gmane.org> --- drivers/dma/mpc512x_dma.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c index df10a48..0053ff8 100644 --- a/drivers/dma/mpc512x_dma.c +++ b/drivers/dma/mpc512x_dma.c @@ -748,11 +748,22 @@ static struct dma_async_tx_descriptor *mpc_dma_prep_slave_sg( static int mpc_dma_device_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd, unsigned long arg) { - struct mpc_dma_chan *mchan = dma_chan_to_mpc_dma_chan(chan); - struct dma_slave_config *cfg = (void *)arg; + struct mpc_dma_chan *mchan; + struct mpc_dma *mdma; + struct dma_slave_config *cfg; + mchan = dma_chan_to_mpc_dma_chan(chan); switch (cmd) { + case DMA_TERMINATE_ALL: + /* disable channel requests */ + mdma = dma_chan_to_mpc_dma(chan); + out_8(&mdma->regs->dmacerq, chan->chan_id); + list_splice_tail_init(&mchan->prepared, &mchan->free); + list_splice_tail_init(&mchan->queued, &mchan->free); + list_splice_tail_init(&mchan->active, &mchan->free); + return 0; case DMA_SLAVE_CONFIG: + cfg = (void *)arg; if (cfg->src_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES && cfg->dst_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES) return -EINVAL; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH RFC 3/8] dma: mpc512x: support 'terminate all' control request @ 2013-07-12 15:26 ` Gerhard Sittig 0 siblings, 0 replies; 45+ messages in thread From: Gerhard Sittig @ 2013-07-12 15:26 UTC (permalink / raw) To: linuxppc-dev, devicetree-discuss, Alexander Popov Cc: Lars-Peter Clausen, Arnd Bergmann, Vinod Koul, Gerhard Sittig, Dan Williams, Anatolij Gustschin implement the TERMINATE_ALL request in the device_control() callback of the DMA engine driver for the MPC512x DMA controller reword variable initialization to better follow the code path and to avoid artificial diffs later on (this style change vanishes when this patch gets squashed with the device_control() routine's introduction) Signed-off-by: Gerhard Sittig <gsi@denx.de> --- drivers/dma/mpc512x_dma.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c index df10a48..0053ff8 100644 --- a/drivers/dma/mpc512x_dma.c +++ b/drivers/dma/mpc512x_dma.c @@ -748,11 +748,22 @@ static struct dma_async_tx_descriptor *mpc_dma_prep_slave_sg( static int mpc_dma_device_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd, unsigned long arg) { - struct mpc_dma_chan *mchan = dma_chan_to_mpc_dma_chan(chan); - struct dma_slave_config *cfg = (void *)arg; + struct mpc_dma_chan *mchan; + struct mpc_dma *mdma; + struct dma_slave_config *cfg; + mchan = dma_chan_to_mpc_dma_chan(chan); switch (cmd) { + case DMA_TERMINATE_ALL: + /* disable channel requests */ + mdma = dma_chan_to_mpc_dma(chan); + out_8(&mdma->regs->dmacerq, chan->chan_id); + list_splice_tail_init(&mchan->prepared, &mchan->free); + list_splice_tail_init(&mchan->queued, &mchan->free); + list_splice_tail_init(&mchan->active, &mchan->free); + return 0; case DMA_SLAVE_CONFIG: + cfg = (void *)arg; if (cfg->src_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES && cfg->dst_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES) return -EINVAL; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH RFC 6/8] dma: of: Add common xlate function for matching by channel id 2013-07-12 15:26 ` Gerhard Sittig @ 2013-07-12 15:26 ` Gerhard Sittig -1 siblings, 0 replies; 45+ messages in thread From: Gerhard Sittig @ 2013-07-12 15:26 UTC (permalink / raw) To: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Alexander Popov Cc: Vinod Koul, Lars-Peter Clausen, Dan Williams From: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org> From: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org> This patch adds a new common OF dma xlate callback function which will match a channel by it's id. The binding expects one integer argument which it will use to lookup the channel by the id. Unlike of_dma_simple_xlate this function is able to handle a system with multiple DMA controllers. When registering the of dma provider with of_dma_controller_register a pointer to the dma_device struct which is associated with the dt node needs to passed as the data parameter. The filter function will use this pointer to match only channels which belong to the specified DMA controller. Signed-off-by: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org> --- drivers/dma/of-dma.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ include/linux/of_dma.h | 4 ++++ 2 files changed, 51 insertions(+) diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c index 7aa0864..d5d528e 100644 --- a/drivers/dma/of-dma.c +++ b/drivers/dma/of-dma.c @@ -229,3 +229,50 @@ struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec, &dma_spec->args[0]); } EXPORT_SYMBOL_GPL(of_dma_simple_xlate); + +struct of_dma_filter_by_chan_id_args { + struct dma_device *dev; + unsigned int chan_id; +}; + +static bool of_dma_filter_by_chan_id(struct dma_chan *chan, void *params) +{ + struct of_dma_filter_by_chan_id_args *args = params; + + return chan->device == args->dev && chan->chan_id == args->chan_id; +} + +/** + * of_dma_xlate_by_chan_id - Translate dt property to DMA channel by channel id + * @dma_spec: pointer to DMA specifier as found in the device tree + * @of_dma: pointer to DMA controller data + * + * This function can be used as the of xlate callback for DMA driver which wants + * to match the channel based on the channel id. When using this xlate function + * the #dma-cells propety of the DMA controller dt node needs to be set to 1. + * The data parameter of of_dma_controller_register must be a pointer to the + * dma_device struct the function should match upon. + * + * Returns pointer to appropriate dma channel on success or NULL on error. + */ +struct dma_chan *of_dma_xlate_by_chan_id(struct of_phandle_args *dma_spec, + struct of_dma *ofdma) +{ + struct of_dma_filter_by_chan_id_args args; + dma_cap_mask_t cap; + + args.dev = ofdma->of_dma_data; + if (!args.dev) + return NULL; + + if (dma_spec->args_count != 1) + return NULL; + + dma_cap_zero(cap); + dma_cap_set(DMA_SLAVE, cap); + + args.chan_id = dma_spec->args[0]; + + return dma_request_channel(cap, of_dma_filter_by_chan_id, &args); +} +EXPORT_SYMBOL_GPL(of_dma_xlate_by_chan_id); diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h index 364dda7..b7cf614 100644 --- a/include/linux/of_dma.h +++ b/include/linux/of_dma.h @@ -42,6 +42,8 @@ extern struct dma_chan *of_dma_request_slave_channel(struct device_node *np, const char *name); extern struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec, struct of_dma *ofdma); +extern struct dma_chan *of_dma_xlate_by_chan_id(struct of_phandle_args *dma_spec, + struct of_dma *ofdma); #else static inline int of_dma_controller_register(struct device_node *np, struct dma_chan *(*of_dma_xlate) @@ -67,6 +69,8 @@ static inline struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_s return NULL; } +#define of_dma_xlate_by_chan_id NULL + #endif #endif /* __LINUX_OF_DMA_H */ -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH RFC 6/8] dma: of: Add common xlate function for matching by channel id @ 2013-07-12 15:26 ` Gerhard Sittig 0 siblings, 0 replies; 45+ messages in thread From: Gerhard Sittig @ 2013-07-12 15:26 UTC (permalink / raw) To: linuxppc-dev, devicetree-discuss, Alexander Popov Cc: Vinod Koul, Lars-Peter Clausen, Anatolij Gustschin, Arnd Bergmann, Dan Williams From: Lars-Peter Clausen <lars@metafoo.de> From: Lars-Peter Clausen <lars@metafoo.de> This patch adds a new common OF dma xlate callback function which will match a channel by it's id. The binding expects one integer argument which it will use to lookup the channel by the id. Unlike of_dma_simple_xlate this function is able to handle a system with multiple DMA controllers. When registering the of dma provider with of_dma_controller_register a pointer to the dma_device struct which is associated with the dt node needs to passed as the data parameter. The filter function will use this pointer to match only channels which belong to the specified DMA controller. Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> --- drivers/dma/of-dma.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ include/linux/of_dma.h | 4 ++++ 2 files changed, 51 insertions(+) diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c index 7aa0864..d5d528e 100644 --- a/drivers/dma/of-dma.c +++ b/drivers/dma/of-dma.c @@ -229,3 +229,50 @@ struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec, &dma_spec->args[0]); } EXPORT_SYMBOL_GPL(of_dma_simple_xlate); + +struct of_dma_filter_by_chan_id_args { + struct dma_device *dev; + unsigned int chan_id; +}; + +static bool of_dma_filter_by_chan_id(struct dma_chan *chan, void *params) +{ + struct of_dma_filter_by_chan_id_args *args = params; + + return chan->device == args->dev && chan->chan_id == args->chan_id; +} + +/** + * of_dma_xlate_by_chan_id - Translate dt property to DMA channel by channel id + * @dma_spec: pointer to DMA specifier as found in the device tree + * @of_dma: pointer to DMA controller data + * + * This function can be used as the of xlate callback for DMA driver which wants + * to match the channel based on the channel id. When using this xlate function + * the #dma-cells propety of the DMA controller dt node needs to be set to 1. + * The data parameter of of_dma_controller_register must be a pointer to the + * dma_device struct the function should match upon. + * + * Returns pointer to appropriate dma channel on success or NULL on error. + */ +struct dma_chan *of_dma_xlate_by_chan_id(struct of_phandle_args *dma_spec, + struct of_dma *ofdma) +{ + struct of_dma_filter_by_chan_id_args args; + dma_cap_mask_t cap; + + args.dev = ofdma->of_dma_data; + if (!args.dev) + return NULL; + + if (dma_spec->args_count != 1) + return NULL; + + dma_cap_zero(cap); + dma_cap_set(DMA_SLAVE, cap); + + args.chan_id = dma_spec->args[0]; + + return dma_request_channel(cap, of_dma_filter_by_chan_id, &args); +} +EXPORT_SYMBOL_GPL(of_dma_xlate_by_chan_id); diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h index 364dda7..b7cf614 100644 --- a/include/linux/of_dma.h +++ b/include/linux/of_dma.h @@ -42,6 +42,8 @@ extern struct dma_chan *of_dma_request_slave_channel(struct device_node *np, const char *name); extern struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec, struct of_dma *ofdma); +extern struct dma_chan *of_dma_xlate_by_chan_id(struct of_phandle_args *dma_spec, + struct of_dma *ofdma); #else static inline int of_dma_controller_register(struct device_node *np, struct dma_chan *(*of_dma_xlate) @@ -67,6 +69,8 @@ static inline struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_s return NULL; } +#define of_dma_xlate_by_chan_id NULL + #endif #endif /* __LINUX_OF_DMA_H */ -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH RFC 0/8] MPC512x DMA slave s/g support, OF DMA lookup 2013-07-12 15:26 ` Gerhard Sittig @ 2013-07-12 16:45 ` Lars-Peter Clausen -1 siblings, 0 replies; 45+ messages in thread From: Lars-Peter Clausen @ 2013-07-12 16:45 UTC (permalink / raw) To: Gerhard Sittig Cc: Vinod Koul, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Alexander Popov, Dan Williams, linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ On 07/12/2013 05:26 PM, Gerhard Sittig wrote: [...] > Lars, there is a checkpatch warning about a line longer than 80 > characters in the common xlate part -- I didn't dare to change your > submission, and the part included here is verbatim from your patchwork > 2331091 (original submission) and 2555701 (resend), is there a followup > which you can provide or point us to? Well the line is 81 characters long, so I wouldn't worry to much about that. But we should probably remove the extern from all the function declarations in dma-of.h, since the extern is kind of implicit for functions declarations. - Lars ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH RFC 0/8] MPC512x DMA slave s/g support, OF DMA lookup @ 2013-07-12 16:45 ` Lars-Peter Clausen 0 siblings, 0 replies; 45+ messages in thread From: Lars-Peter Clausen @ 2013-07-12 16:45 UTC (permalink / raw) To: Gerhard Sittig Cc: Arnd Bergmann, Vinod Koul, devicetree-discuss, Alexander Popov, Dan Williams, Anatolij Gustschin, linuxppc-dev On 07/12/2013 05:26 PM, Gerhard Sittig wrote: [...] > Lars, there is a checkpatch warning about a line longer than 80 > characters in the common xlate part -- I didn't dare to change your > submission, and the part included here is verbatim from your patchwork > 2331091 (original submission) and 2555701 (resend), is there a followup > which you can provide or point us to? Well the line is 81 characters long, so I wouldn't worry to much about that. But we should probably remove the extern from all the function declarations in dma-of.h, since the extern is kind of implicit for functions declarations. - Lars ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH RFC v2 0/5] MPC512x DMA slave s/g support, OF DMA lookup 2013-07-12 15:26 ` Gerhard Sittig @ 2013-07-14 12:01 ` Gerhard Sittig -1 siblings, 0 replies; 45+ messages in thread From: Gerhard Sittig @ 2013-07-14 12:01 UTC (permalink / raw) To: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Alexander Popov Cc: Lars-Peter Clausen, Vinod Koul, Dan Williams this series - introduces slave s/g support (that's support for DMA transfers which involve peripherals in contrast to mem-to-mem transfers) - adds device tree based lookup support for DMA channels - combines floating patches and related feedback which already covered several aspects of what the suggested LPB driver needs, to demonstrate how integration might be done - carries Q&D SD card support to enable another DMA client during test, while this patch needs to get dropped upon pickup changes since v1: - re-order mpc8308 related code paths for improved readability, no change in behaviour, introduction of symbolic channel names here already - squash 'execute() start condition' and 'terminate all' into the introduction of 'slave s/g prep' and 'device control' support; refuse s/g lists with more than one item since slave support is operational yet proper s/g support is missing (can get addressed later) - always start transfers from software on MPC8308 as there are no external request lines for peripheral flow control - drop dt-bindings header file and symbolic channel names in OF nodes known issues: - it's yet to get confirmed whether MPC8308 can use slave support or whether the DMA controller's driver shall actively reject it, the information that's available so far suggests that peripheral transfers to IP bus attached I/O is useful and shall not get blocked right away - currently encoded constraints do work for SD card and LPB test suite (all known DMA clients ATM), but will need more tuning or support for automatic adjustment for transfers of arbitrary length oh, and I'd like to get feedback on whether attribution handling is appropriate, as I had to squash patches from several authors to not break bisectability (compilation had worked, but the code had not run successfully on all previously supported hardware) Alexander, when you pickup this series and improve slave s/g support and adjust it for more general use, you have my OK to incorporate and further adjust the parts that I've contributed Alexander Popov (1): dma: mpc512x: add support for peripheral transfers Gerhard Sittig (3): dma: mpc512x: re-order mpc8308 specific instructions dma: mpc512x: register for device tree channel lookup HACK mmc: mxcmmc: enable clocks for the MPC512x Lars-Peter Clausen (1): dma: of: Add common xlate function for matching by channel id .../devicetree/bindings/dma/mpc512x-dma.txt | 55 +++++ arch/powerpc/boot/dts/mpc5121.dtsi | 1 + drivers/dma/mpc512x_dma.c | 237 +++++++++++++++++--- drivers/dma/of-dma.c | 47 ++++ drivers/mmc/host/mxcmmc.c | 41 ++-- include/linux/of_dma.h | 4 + 6 files changed, 344 insertions(+), 41 deletions(-) create mode 100644 Documentation/devicetree/bindings/dma/mpc512x-dma.txt -- 1.7.10.4 ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH RFC v2 0/5] MPC512x DMA slave s/g support, OF DMA lookup @ 2013-07-14 12:01 ` Gerhard Sittig 0 siblings, 0 replies; 45+ messages in thread From: Gerhard Sittig @ 2013-07-14 12:01 UTC (permalink / raw) To: linuxppc-dev, devicetree-discuss, Alexander Popov Cc: Lars-Peter Clausen, Arnd Bergmann, Vinod Koul, Gerhard Sittig, Dan Williams, Anatolij Gustschin this series - introduces slave s/g support (that's support for DMA transfers which involve peripherals in contrast to mem-to-mem transfers) - adds device tree based lookup support for DMA channels - combines floating patches and related feedback which already covered several aspects of what the suggested LPB driver needs, to demonstrate how integration might be done - carries Q&D SD card support to enable another DMA client during test, while this patch needs to get dropped upon pickup changes since v1: - re-order mpc8308 related code paths for improved readability, no change in behaviour, introduction of symbolic channel names here already - squash 'execute() start condition' and 'terminate all' into the introduction of 'slave s/g prep' and 'device control' support; refuse s/g lists with more than one item since slave support is operational yet proper s/g support is missing (can get addressed later) - always start transfers from software on MPC8308 as there are no external request lines for peripheral flow control - drop dt-bindings header file and symbolic channel names in OF nodes known issues: - it's yet to get confirmed whether MPC8308 can use slave support or whether the DMA controller's driver shall actively reject it, the information that's available so far suggests that peripheral transfers to IP bus attached I/O is useful and shall not get blocked right away - currently encoded constraints do work for SD card and LPB test suite (all known DMA clients ATM), but will need more tuning or support for automatic adjustment for transfers of arbitrary length oh, and I'd like to get feedback on whether attribution handling is appropriate, as I had to squash patches from several authors to not break bisectability (compilation had worked, but the code had not run successfully on all previously supported hardware) Alexander, when you pickup this series and improve slave s/g support and adjust it for more general use, you have my OK to incorporate and further adjust the parts that I've contributed Alexander Popov (1): dma: mpc512x: add support for peripheral transfers Gerhard Sittig (3): dma: mpc512x: re-order mpc8308 specific instructions dma: mpc512x: register for device tree channel lookup HACK mmc: mxcmmc: enable clocks for the MPC512x Lars-Peter Clausen (1): dma: of: Add common xlate function for matching by channel id .../devicetree/bindings/dma/mpc512x-dma.txt | 55 +++++ arch/powerpc/boot/dts/mpc5121.dtsi | 1 + drivers/dma/mpc512x_dma.c | 237 +++++++++++++++++--- drivers/dma/of-dma.c | 47 ++++ drivers/mmc/host/mxcmmc.c | 41 ++-- include/linux/of_dma.h | 4 + 6 files changed, 344 insertions(+), 41 deletions(-) create mode 100644 Documentation/devicetree/bindings/dma/mpc512x-dma.txt -- 1.7.10.4 ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH RFC v2 1/5] dma: mpc512x: re-order mpc8308 specific instructions 2013-07-14 12:01 ` Gerhard Sittig (?) @ 2013-07-14 12:01 ` Gerhard Sittig 2013-08-12 13:38 ` Alexander Popov -1 siblings, 1 reply; 45+ messages in thread From: Gerhard Sittig @ 2013-07-14 12:01 UTC (permalink / raw) To: linuxppc-dev, devicetree-discuss, Alexander Popov Cc: Lars-Peter Clausen, Arnd Bergmann, Vinod Koul, Gerhard Sittig, Dan Williams, Anatolij Gustschin it's rather unexpected to have the MPC8308 specific code in the 'else' branch so distant from the "is MPC8308?" check concentrate the test and the specific code for MPC8308 in the 'if' branch and handle MPC512x in the 'else' branch; use a symbolic channel count which in combination with the re-ordering obsoletes a comment this modification only re-orders instructions but doesn't change behaviour Signed-off-by: Gerhard Sittig <gsi@denx.de> --- drivers/dma/mpc512x_dma.c | 48 +++++++++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c index 2d95673..4e03f1e 100644 --- a/drivers/dma/mpc512x_dma.c +++ b/drivers/dma/mpc512x_dma.c @@ -50,9 +50,23 @@ #define MPC_DMA_DESCRIPTORS 64 /* Macro definitions */ -#define MPC_DMA_CHANNELS 64 #define MPC_DMA_TCD_OFFSET 0x1000 +/* + * the maximum channel count, and specific channels which need + * special processing, for individual hardware variants + * + * and the maximum channel count over all supported controllers, + * used for data structure sizes + */ +enum mpc8308_dmachan_id_t { + MPC8308_DMACHAN_MAX = 16, +}; +enum mpc512x_dmachan_id_t { + MPC512x_DMACHAN_MAX = 64, +}; +#define MPC_DMA_CHANNELS 64 + /* Arbitration mode of group and channel */ #define MPC_DMA_DMACR_EDCG (1 << 31) #define MPC_DMA_DMACR_ERGA (1 << 3) @@ -716,10 +730,10 @@ static int mpc_dma_probe(struct platform_device *op) dma = &mdma->dma; dma->dev = dev; - if (!mdma->is_mpc8308) - dma->chancnt = MPC_DMA_CHANNELS; + if (mdma->is_mpc8308) + dma->chancnt = MPC8308_DMACHAN_MAX; else - dma->chancnt = 16; /* MPC8308 DMA has only 16 channels */ + dma->chancnt = MPC512x_DMACHAN_MAX; dma->device_alloc_chan_resources = mpc_dma_alloc_chan_resources; dma->device_free_chan_resources = mpc_dma_free_chan_resources; dma->device_issue_pending = mpc_dma_issue_pending; @@ -753,7 +767,19 @@ static int mpc_dma_probe(struct platform_device *op) * - Round-robin group arbitration, * - Round-robin channel arbitration. */ - if (!mdma->is_mpc8308) { + if (mdma->is_mpc8308) { + /* MPC8308 has 16 channels and lacks some registers */ + out_be32(&mdma->regs->dmacr, MPC_DMA_DMACR_ERCA); + + /* enable snooping */ + out_be32(&mdma->regs->dmagpor, MPC_DMA_DMAGPOR_SNOOP_ENABLE); + /* Disable error interrupts */ + out_be32(&mdma->regs->dmaeeil, 0); + + /* Clear interrupts status */ + out_be32(&mdma->regs->dmaintl, 0xFFFF); + out_be32(&mdma->regs->dmaerrl, 0xFFFF); + } else { out_be32(&mdma->regs->dmacr, MPC_DMA_DMACR_EDCG | MPC_DMA_DMACR_ERGA | MPC_DMA_DMACR_ERCA); @@ -774,18 +800,6 @@ static int mpc_dma_probe(struct platform_device *op) /* Route interrupts to IPIC */ out_be32(&mdma->regs->dmaihsa, 0); out_be32(&mdma->regs->dmailsa, 0); - } else { - /* MPC8308 has 16 channels and lacks some registers */ - out_be32(&mdma->regs->dmacr, MPC_DMA_DMACR_ERCA); - - /* enable snooping */ - out_be32(&mdma->regs->dmagpor, MPC_DMA_DMAGPOR_SNOOP_ENABLE); - /* Disable error interrupts */ - out_be32(&mdma->regs->dmaeeil, 0); - - /* Clear interrupts status */ - out_be32(&mdma->regs->dmaintl, 0xFFFF); - out_be32(&mdma->regs->dmaerrl, 0xFFFF); } /* Register DMA engine */ -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH RFC v2 1/5] dma: mpc512x: re-order mpc8308 specific instructions 2013-07-14 12:01 ` [PATCH RFC v2 1/5] dma: mpc512x: re-order mpc8308 specific instructions Gerhard Sittig @ 2013-08-12 13:38 ` Alexander Popov 0 siblings, 0 replies; 45+ messages in thread From: Alexander Popov @ 2013-08-12 13:38 UTC (permalink / raw) To: Gerhard Sittig Cc: linuxppc-dev, Dan Williams, Vinod Koul, Lars-Peter Clausen, Arnd Bergmann, Anatolij Gustschin, linux-kernel Hello everyone! 2013/7/14 Gerhard Sittig <gsi@denx.de>: > @@ -50,9 +50,23 @@ > #define MPC_DMA_DESCRIPTORS 64 > > /* Macro definitions */ > -#define MPC_DMA_CHANNELS 64 > #define MPC_DMA_TCD_OFFSET 0x1000 > > +/* > + * the maximum channel count, and specific channels which need > + * special processing, for individual hardware variants > + * > + * and the maximum channel count over all supported controllers, > + * used for data structure sizes > + */ > +enum mpc8308_dmachan_id_t { > + MPC8308_DMACHAN_MAX = 16, > +}; > +enum mpc512x_dmachan_id_t { > + MPC512x_DMACHAN_MAX = 64, > +}; > +#define MPC_DMA_CHANNELS 64 > + I offer to use #define instead of enum here since individual channels don't require special handling. Best regards, Alexander. ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH RFC v2 1/5] dma: mpc512x: re-order mpc8308 specific instructions @ 2013-08-12 13:38 ` Alexander Popov 0 siblings, 0 replies; 45+ messages in thread From: Alexander Popov @ 2013-08-12 13:38 UTC (permalink / raw) To: Gerhard Sittig Cc: Lars-Peter Clausen, Arnd Bergmann, Vinod Koul, linux-kernel, Dan Williams, Anatolij Gustschin, linuxppc-dev Hello everyone! 2013/7/14 Gerhard Sittig <gsi@denx.de>: > @@ -50,9 +50,23 @@ > #define MPC_DMA_DESCRIPTORS 64 > > /* Macro definitions */ > -#define MPC_DMA_CHANNELS 64 > #define MPC_DMA_TCD_OFFSET 0x1000 > > +/* > + * the maximum channel count, and specific channels which need > + * special processing, for individual hardware variants > + * > + * and the maximum channel count over all supported controllers, > + * used for data structure sizes > + */ > +enum mpc8308_dmachan_id_t { > + MPC8308_DMACHAN_MAX = 16, > +}; > +enum mpc512x_dmachan_id_t { > + MPC512x_DMACHAN_MAX = 64, > +}; > +#define MPC_DMA_CHANNELS 64 > + I offer to use #define instead of enum here since individual channels don't require special handling. Best regards, Alexander. ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH RFC v2 2/5] dma: mpc512x: add support for peripheral transfers 2013-07-14 12:01 ` Gerhard Sittig (?) (?) @ 2013-07-14 12:01 ` Gerhard Sittig 2013-07-16 10:37 ` Lars-Peter Clausen 2013-08-12 13:37 ` Alexander Popov -1 siblings, 2 replies; 45+ messages in thread From: Gerhard Sittig @ 2013-07-14 12:01 UTC (permalink / raw) To: linuxppc-dev, devicetree-discuss, Alexander Popov Cc: Lars-Peter Clausen, Arnd Bergmann, Vinod Koul, Gerhard Sittig, Dan Williams, Anatolij Gustschin From: Alexander Popov <a13xp0p0v88@gmail.com> introduce support for slave s/g transfer preparation and the associated device control callback in the MPC512x DMA controller driver, which adds support for data transfers between memory and peripheral I/O to the previously supported mem-to-mem transfers refuse to prepare chunked transfers (transfers with more than one part) as long as proper support for scatter/gather is lacking keep MPC8308 operational by always starting transfers from software, this SoC appears to not have request lines for flow control when peripherals are involved in transfers [ introduction of slave s/g preparation and device control ] Signed-off-by: Alexander Popov <a13xp0p0v88@gmail.com> [ execute() start condition, mpc8308 compat, terminate all, s/g length check, reworded commit msg ] Signed-off-by: Gerhard Sittig <gsi@denx.de> --- drivers/dma/mpc512x_dma.c | 168 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 161 insertions(+), 7 deletions(-) diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c index 4e03f1e..55e6a87 100644 --- a/drivers/dma/mpc512x_dma.c +++ b/drivers/dma/mpc512x_dma.c @@ -2,6 +2,7 @@ * Copyright (C) Freescale Semicondutor, Inc. 2007, 2008. * Copyright (C) Semihalf 2009 * Copyright (C) Ilya Yanok, Emcraft Systems 2010 + * Copyright (C) Alexander Popov, Promcontroller 2013 * * Written by Piotr Ziecik <kosmo@semihalf.com>. Hardware description * (defines, structures and comments) was taken from MPC5121 DMA driver @@ -28,11 +29,6 @@ * file called COPYING. */ -/* - * This is initial version of MPC5121 DMA driver. Only memory to memory - * transfers are supported (tested using dmatest module). - */ - #include <linux/module.h> #include <linux/dmaengine.h> #include <linux/dma-mapping.h> @@ -63,6 +59,7 @@ enum mpc8308_dmachan_id_t { MPC8308_DMACHAN_MAX = 16, }; enum mpc512x_dmachan_id_t { + MPC512x_DMACHAN_MDDRC = 32, MPC512x_DMACHAN_MAX = 64, }; #define MPC_DMA_CHANNELS 64 @@ -204,9 +201,13 @@ struct mpc_dma_chan { struct list_head completed; struct mpc_dma_tcd *tcd; dma_addr_t tcd_paddr; + u32 tcd_nunits; /* Lock for this structure */ spinlock_t lock; + + /* Channel's peripheral fifo address */ + dma_addr_t per_paddr; }; struct mpc_dma { @@ -270,7 +271,12 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan) prev->tcd->dlast_sga = mdesc->tcd_paddr; prev->tcd->e_sg = 1; - mdesc->tcd->start = 1; + + /* software start for mem-to-mem transfers */ + if (mdma->is_mpc8308) + mdesc->tcd->start = 1; + else if (cid == MPC512x_DMACHAN_MDDRC) + mdesc->tcd->start = 1; prev = mdesc; } @@ -282,7 +288,17 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan) if (first != prev) mdma->tcd[cid].e_sg = 1; - out_8(&mdma->regs->dmassrt, cid); + + if (mdma->is_mpc8308) { + /* MPC8308, no request lines, software initiated start */ + out_8(&mdma->regs->dmassrt, cid); + } else if (cid == MPC512x_DMACHAN_MDDRC) { + /* memory to memory transfer, software initiated start */ + out_8(&mdma->regs->dmassrt, cid); + } else { + /* peripherals involved, use external request line */ + out_8(&mdma->regs->dmaserq, cid); + } } /* Handle interrupt on one half of DMA controller (32 channels) */ @@ -655,6 +671,141 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src, return &mdesc->desc; } +static struct dma_async_tx_descriptor *mpc_dma_prep_slave_sg( + struct dma_chan *chan, struct scatterlist *sgl, + unsigned int sg_len, enum dma_transfer_direction direction, + unsigned long flags, void *context) +{ + struct mpc_dma *mdma = dma_chan_to_mpc_dma(chan); + struct mpc_dma_chan *mchan = dma_chan_to_mpc_dma_chan(chan); + struct mpc_dma_desc *mdesc = NULL; + struct mpc_dma_tcd *tcd; + unsigned long iflags; + struct scatterlist *sg; + size_t len; + int iter, i; + + if (!list_empty(&mchan->active)) + return NULL; + + /* currently there is no proper support for scatter/gather */ + if (sg_len > 1) + return NULL; + + for_each_sg(sgl, sg, sg_len, i) { + spin_lock_irqsave(&mchan->lock, iflags); + + mdesc = list_first_entry(&mchan->free, struct mpc_dma_desc, + node); + if (!mdesc) { + spin_unlock_irqrestore(&mchan->lock, iflags); + /* try to free completed descriptors */ + mpc_dma_process_completed(mdma); + return NULL; + } + + list_del(&mdesc->node); + + spin_unlock_irqrestore(&mchan->lock, iflags); + + mdesc->error = 0; + tcd = mdesc->tcd; + + /* Prepare Transfer Control Descriptor for this transaction */ + memset(tcd, 0, sizeof(struct mpc_dma_tcd)); + + if (!IS_ALIGNED(sg_dma_address(sg), 4)) + return NULL; + + if (direction == DMA_DEV_TO_MEM) { + tcd->saddr = mchan->per_paddr; + tcd->daddr = sg_dma_address(sg); + tcd->soff = 0; + tcd->doff = 4; + } else if (direction == DMA_MEM_TO_DEV) { + tcd->saddr = sg_dma_address(sg); + tcd->daddr = mchan->per_paddr; + tcd->soff = 4; + tcd->doff = 0; + } else { + return NULL; + } + tcd->ssize = MPC_DMA_TSIZE_4; + tcd->dsize = MPC_DMA_TSIZE_4; + + len = sg_dma_len(sg); + + if (mchan->tcd_nunits) + tcd->nbytes = mchan->tcd_nunits * 4; + else + tcd->nbytes = 64; + + if (!IS_ALIGNED(len, tcd->nbytes)) + return NULL; + + iter = len / tcd->nbytes; + if (iter > ((1 << 15) - 1)) { /* maximum biter */ + return NULL; /* len is too big */ + } else { + /* citer_linkch contains the high bits of iter */ + tcd->biter = iter & 0x1ff; + tcd->biter_linkch = iter >> 9; + tcd->citer = tcd->biter; + tcd->citer_linkch = tcd->biter_linkch; + } + + tcd->e_sg = 0; + tcd->d_req = 1; + + /* Place descriptor in prepared list */ + spin_lock_irqsave(&mchan->lock, iflags); + list_add_tail(&mdesc->node, &mchan->prepared); + spin_unlock_irqrestore(&mchan->lock, iflags); + } + + /* Return the last descriptor */ + return &mdesc->desc; +} + +static int mpc_dma_device_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd, + unsigned long arg) +{ + struct mpc_dma_chan *mchan; + struct mpc_dma *mdma; + struct dma_slave_config *cfg; + + mchan = dma_chan_to_mpc_dma_chan(chan); + switch (cmd) { + case DMA_TERMINATE_ALL: + /* disable channel requests */ + mdma = dma_chan_to_mpc_dma(chan); + out_8(&mdma->regs->dmacerq, chan->chan_id); + list_splice_tail_init(&mchan->prepared, &mchan->free); + list_splice_tail_init(&mchan->queued, &mchan->free); + list_splice_tail_init(&mchan->active, &mchan->free); + return 0; + case DMA_SLAVE_CONFIG: + cfg = (void *)arg; + if (cfg->src_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES && + cfg->dst_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES) + return -EINVAL; + + if (cfg->direction == DMA_DEV_TO_MEM) { + mchan->per_paddr = cfg->src_addr; + mchan->tcd_nunits = cfg->src_maxburst; + } else { + mchan->per_paddr = cfg->dst_addr; + mchan->tcd_nunits = cfg->dst_maxburst; + } + + return 0; + default: + return -ENOSYS; + } + + return -EINVAL; +} + static int mpc_dma_probe(struct platform_device *op) { struct device_node *dn = op->dev.of_node; @@ -739,9 +890,12 @@ static int mpc_dma_probe(struct platform_device *op) dma->device_issue_pending = mpc_dma_issue_pending; dma->device_tx_status = mpc_dma_tx_status; dma->device_prep_dma_memcpy = mpc_dma_prep_memcpy; + dma->device_prep_slave_sg = mpc_dma_prep_slave_sg; + dma->device_control = mpc_dma_device_control; INIT_LIST_HEAD(&dma->channels); dma_cap_set(DMA_MEMCPY, dma->cap_mask); + dma_cap_set(DMA_SLAVE, dma->cap_mask); for (i = 0; i < dma->chancnt; i++) { mchan = &mdma->channels[i]; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH RFC v2 2/5] dma: mpc512x: add support for peripheral transfers 2013-07-14 12:01 ` [PATCH RFC v2 2/5] dma: mpc512x: add support for peripheral transfers Gerhard Sittig @ 2013-07-16 10:37 ` Lars-Peter Clausen [not found] ` <51E5226D.9050100-Qo5EllUWu/uELgA04lAiVw@public.gmane.org> 2013-08-12 13:37 ` Alexander Popov 1 sibling, 1 reply; 45+ messages in thread From: Lars-Peter Clausen @ 2013-07-16 10:37 UTC (permalink / raw) To: Gerhard Sittig Cc: Arnd Bergmann, Vinod Koul, devicetree-discuss, Alexander Popov, Dan Williams, Anatolij Gustschin, linuxppc-dev On 07/14/2013 02:01 PM, Gerhard Sittig wrote: > From: Alexander Popov <a13xp0p0v88@gmail.com> > > introduce support for slave s/g transfer preparation and the associated > device control callback in the MPC512x DMA controller driver, which adds > support for data transfers between memory and peripheral I/O to the > previously supported mem-to-mem transfers > > refuse to prepare chunked transfers (transfers with more than one part) > as long as proper support for scatter/gather is lacking > > keep MPC8308 operational by always starting transfers from software, > this SoC appears to not have request lines for flow control when > peripherals are involved in transfers I had a look at the current driver and it seems that any channel can be used for memcpy operation not only the MDDRC channel. Since the dmaengine API will just pick one of the currently free channels when performing a memcpy operation I think this patch breaks memcpy operations. You probably need to register two dma controllers, one for memcpy operations one for slave operations, that way you can ensure that only the MDDRC channel is used for memcpy operations. > > [ introduction of slave s/g preparation and device control ] > Signed-off-by: Alexander Popov <a13xp0p0v88@gmail.com> > [ execute() start condition, mpc8308 compat, terminate all, s/g length check, reworded commit msg ] > Signed-off-by: Gerhard Sittig <gsi@denx.de> > --- [...] > + > +static int mpc_dma_device_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd, > + unsigned long arg) > +{ > + struct mpc_dma_chan *mchan; > + struct mpc_dma *mdma; > + struct dma_slave_config *cfg; > + > + mchan = dma_chan_to_mpc_dma_chan(chan); > + switch (cmd) { > + case DMA_TERMINATE_ALL: > + /* disable channel requests */ > + mdma = dma_chan_to_mpc_dma(chan); > + out_8(&mdma->regs->dmacerq, chan->chan_id); > + list_splice_tail_init(&mchan->prepared, &mchan->free); > + list_splice_tail_init(&mchan->queued, &mchan->free); > + list_splice_tail_init(&mchan->active, &mchan->free); This probably need locking. > + return 0; [...] > +} > + ^ permalink raw reply [flat|nested] 45+ messages in thread
[parent not found: <51E5226D.9050100-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>]
* Re: [PATCH RFC v2 2/5] dma: mpc512x: add support for peripheral transfers 2013-07-16 10:37 ` Lars-Peter Clausen @ 2013-07-17 10:42 ` Gerhard Sittig 0 siblings, 0 replies; 45+ messages in thread From: Gerhard Sittig @ 2013-07-17 10:42 UTC (permalink / raw) To: Lars-Peter Clausen Cc: Vinod Koul, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Alexander Popov, Dan Williams, linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ On Tue, Jul 16, 2013 at 12:37 +0200, Lars-Peter Clausen wrote: > > On 07/14/2013 02:01 PM, Gerhard Sittig wrote: > > From: Alexander Popov <a13xp0p0v88-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > > > > introduce support for slave s/g transfer preparation and the associated > > device control callback in the MPC512x DMA controller driver, which adds > > support for data transfers between memory and peripheral I/O to the > > previously supported mem-to-mem transfers > > > > refuse to prepare chunked transfers (transfers with more than one part) > > as long as proper support for scatter/gather is lacking > > > > keep MPC8308 operational by always starting transfers from software, > > this SoC appears to not have request lines for flow control when > > peripherals are involved in transfers > > I had a look at the current driver and it seems that any channel can be used > for memcpy operation not only the MDDRC channel. Since the dmaengine API > will just pick one of the currently free channels when performing a memcpy > operation I think this patch breaks memcpy operations. You probably need to > register two dma controllers, one for memcpy operations one for slave > operations, that way you can ensure that only the MDDRC channel is used for > memcpy operations. OK, so the need for explicit start in software or external request by the peripheral remains, but the condition for the choice is different. It might become a flag attached to the DMA channel, that gets setup in the prep routines (memory and slave access each use their own prep calls) and gets evaluated in execute. Something like "will access memory", or "will access peripheral" (the latter may be preferrable since slave support is the new feature). This assumes that the non-MDDRC channels can get used for memory transfers as well, which your description of the the former use suggests, when slave support was absent. Making the MDDRC channel preferrable for memcpy operations is orthogonal to that. > > > > [ introduction of slave s/g preparation and device control ] > > Signed-off-by: Alexander Popov <a13xp0p0v88-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > > [ execute() start condition, mpc8308 compat, terminate all, s/g length check, reworded commit msg ] > > Signed-off-by: Gerhard Sittig <gsi-ynQEQJNshbs@public.gmane.org> > > --- > [...] > > + > > +static int mpc_dma_device_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd, > > + unsigned long arg) > > +{ > > + struct mpc_dma_chan *mchan; > > + struct mpc_dma *mdma; > > + struct dma_slave_config *cfg; > > + > > + mchan = dma_chan_to_mpc_dma_chan(chan); > > + switch (cmd) { > > + case DMA_TERMINATE_ALL: > > + /* disable channel requests */ > > + mdma = dma_chan_to_mpc_dma(chan); > > + out_8(&mdma->regs->dmacerq, chan->chan_id); > > + list_splice_tail_init(&mchan->prepared, &mchan->free); > > + list_splice_tail_init(&mchan->queued, &mchan->free); > > + list_splice_tail_init(&mchan->active, &mchan->free); > > This probably need locking. Ah, yes. It needs to grab the same lock as all the other list manipulations in the driver's source. virtually yours Gerhard Sittig -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office-ynQEQJNshbs@public.gmane.org ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH RFC v2 2/5] dma: mpc512x: add support for peripheral transfers @ 2013-07-17 10:42 ` Gerhard Sittig 0 siblings, 0 replies; 45+ messages in thread From: Gerhard Sittig @ 2013-07-17 10:42 UTC (permalink / raw) To: Lars-Peter Clausen Cc: Arnd Bergmann, Vinod Koul, devicetree-discuss, Alexander Popov, Dan Williams, Anatolij Gustschin, linuxppc-dev On Tue, Jul 16, 2013 at 12:37 +0200, Lars-Peter Clausen wrote: > > On 07/14/2013 02:01 PM, Gerhard Sittig wrote: > > From: Alexander Popov <a13xp0p0v88@gmail.com> > > > > introduce support for slave s/g transfer preparation and the associated > > device control callback in the MPC512x DMA controller driver, which adds > > support for data transfers between memory and peripheral I/O to the > > previously supported mem-to-mem transfers > > > > refuse to prepare chunked transfers (transfers with more than one part) > > as long as proper support for scatter/gather is lacking > > > > keep MPC8308 operational by always starting transfers from software, > > this SoC appears to not have request lines for flow control when > > peripherals are involved in transfers > > I had a look at the current driver and it seems that any channel can be used > for memcpy operation not only the MDDRC channel. Since the dmaengine API > will just pick one of the currently free channels when performing a memcpy > operation I think this patch breaks memcpy operations. You probably need to > register two dma controllers, one for memcpy operations one for slave > operations, that way you can ensure that only the MDDRC channel is used for > memcpy operations. OK, so the need for explicit start in software or external request by the peripheral remains, but the condition for the choice is different. It might become a flag attached to the DMA channel, that gets setup in the prep routines (memory and slave access each use their own prep calls) and gets evaluated in execute. Something like "will access memory", or "will access peripheral" (the latter may be preferrable since slave support is the new feature). This assumes that the non-MDDRC channels can get used for memory transfers as well, which your description of the the former use suggests, when slave support was absent. Making the MDDRC channel preferrable for memcpy operations is orthogonal to that. > > > > [ introduction of slave s/g preparation and device control ] > > Signed-off-by: Alexander Popov <a13xp0p0v88@gmail.com> > > [ execute() start condition, mpc8308 compat, terminate all, s/g length check, reworded commit msg ] > > Signed-off-by: Gerhard Sittig <gsi@denx.de> > > --- > [...] > > + > > +static int mpc_dma_device_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd, > > + unsigned long arg) > > +{ > > + struct mpc_dma_chan *mchan; > > + struct mpc_dma *mdma; > > + struct dma_slave_config *cfg; > > + > > + mchan = dma_chan_to_mpc_dma_chan(chan); > > + switch (cmd) { > > + case DMA_TERMINATE_ALL: > > + /* disable channel requests */ > > + mdma = dma_chan_to_mpc_dma(chan); > > + out_8(&mdma->regs->dmacerq, chan->chan_id); > > + list_splice_tail_init(&mchan->prepared, &mchan->free); > > + list_splice_tail_init(&mchan->queued, &mchan->free); > > + list_splice_tail_init(&mchan->active, &mchan->free); > > This probably need locking. Ah, yes. It needs to grab the same lock as all the other list manipulations in the driver's source. virtually yours Gerhard Sittig -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH RFC v2 2/5] dma: mpc512x: add support for peripheral transfers 2013-07-17 10:42 ` Gerhard Sittig (?) @ 2013-07-31 7:46 ` Alexander Popov -1 siblings, 0 replies; 45+ messages in thread From: Alexander Popov @ 2013-07-31 7:46 UTC (permalink / raw) To: Lars-Peter Clausen, linuxppc-dev, devicetree-discuss, Alexander Popov, Dan Williams, Vinod Koul, Arnd Bergmann, Anatolij Gustschin Hello everyone! I've just sent v3 of part 1 and 2 of RFC series: https://patchwork.kernel.org/patch/2836123/ https://patchwork.kernel.org/patch/2836124/ 2013/7/17 Gerhard Sittig <gsi@denx.de>: > OK, so the need for explicit start in software or external > request by the peripheral remains, but the condition for the > choice is different. It might become a flag attached to the DMA > channel, that gets setup in the prep routines (memory and slave > access each use their own prep calls) and gets evaluated in > execute. I made a flag "will_access_peripheral" be set up in the device control callback. > non-MDDRC channels can get used for memory > transfers as well, which your description of the the former use > suggests, when slave support was absent. I tested v3 on MPC5125 with SCLPC driver (transfers between dev and mem work fine) and dmatest module (all 64 DMA channels can perform mem-to-mem transfers). >> > + mchan = dma_chan_to_mpc_dma_chan(chan); >> > + switch (cmd) { >> > + case DMA_TERMINATE_ALL: >> > + /* disable channel requests */ >> > + mdma = dma_chan_to_mpc_dma(chan); >> > + out_8(&mdma->regs->dmacerq, chan->chan_id); >> > + list_splice_tail_init(&mchan->prepared, &mchan->free); >> > + list_splice_tail_init(&mchan->queued, &mchan->free); >> > + list_splice_tail_init(&mchan->active, &mchan->free); >> >> This probably need locking. > > Ah, yes. It needs to grab the same lock as all the other list > manipulations in the driver's source. Fixed that too. Best regards, Alexander. ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH RFC v2 2/5] dma: mpc512x: add support for peripheral transfers 2013-07-14 12:01 ` [PATCH RFC v2 2/5] dma: mpc512x: add support for peripheral transfers Gerhard Sittig @ 2013-08-12 13:37 ` Alexander Popov 2013-08-12 13:37 ` Alexander Popov 1 sibling, 0 replies; 45+ messages in thread From: Alexander Popov @ 2013-08-12 13:37 UTC (permalink / raw) To: Gerhard Sittig Cc: linuxppc-dev, Dan Williams, Vinod Koul, Lars-Peter Clausen, Arnd Bergmann, Anatolij Gustschin, linux-kernel Hello everyone! Changes offered in this letter: - Adding a flag "will_access_peripheral" to DMA transfer descriptor according recommendations of Gerhard Sittig. This flag is set in mpc_dma_prep_memcpy() and mpc_dma_prep_slave_sg() and is evaluated in mpc_dma_execute(). - Adding locking and removing default nbytes value according recommendations of Lars-Peter Clausen. I tested these changes on MPC5125 with SCLPC driver (transfers between dev and mem work fine) and dmatest module (all 64 DMA channels can perform mem-to-mem transfers which can be chained in one DMA transaction). 2013/7/14 Gerhard Sittig <gsi@denx.de>: > From: Alexander Popov <a13xp0p0v88@gmail.com> > > introduce support for slave s/g transfer preparation and the associated > device control callback in the MPC512x DMA controller driver, which adds > support for data transfers between memory and peripheral I/O to the > previously supported mem-to-mem transfers > > refuse to prepare chunked transfers (transfers with more than one part) > as long as proper support for scatter/gather is lacking > > keep MPC8308 operational by always starting transfers from software, > this SoC appears to not have request lines for flow control when > peripherals are involved in transfers > > [ introduction of slave s/g preparation and device control ] > Signed-off-by: Alexander Popov <a13xp0p0v88@gmail.com> > [ execute() start condition, mpc8308 compat, terminate all, s/g length check, reworded commit msg ] > Signed-off-by: Gerhard Sittig <gsi@denx.de> > --- > drivers/dma/mpc512x_dma.c | 168 +++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 161 insertions(+), 7 deletions(-) > > diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c > index 4e03f1e..55e6a87 100644 > --- a/drivers/dma/mpc512x_dma.c > +++ b/drivers/dma/mpc512x_dma.c > @@ -2,6 +2,7 @@ > * Copyright (C) Freescale Semicondutor, Inc. 2007, 2008. > * Copyright (C) Semihalf 2009 > * Copyright (C) Ilya Yanok, Emcraft Systems 2010 > + * Copyright (C) Alexander Popov, Promcontroller 2013 > * > * Written by Piotr Ziecik <kosmo@semihalf.com>. Hardware description > * (defines, structures and comments) was taken from MPC5121 DMA driver > @@ -28,11 +29,6 @@ > * file called COPYING. > */ > > -/* > - * This is initial version of MPC5121 DMA driver. Only memory to memory > - * transfers are supported (tested using dmatest module). > - */ > - > #include <linux/module.h> > #include <linux/dmaengine.h> > #include <linux/dma-mapping.h> > @@ -63,6 +59,7 @@ enum mpc8308_dmachan_id_t { > MPC8308_DMACHAN_MAX = 16, > }; > enum mpc512x_dmachan_id_t { > + MPC512x_DMACHAN_MDDRC = 32, > MPC512x_DMACHAN_MAX = 64, > }; > #define MPC_DMA_CHANNELS 64 Adding the flag: @@ -193,6 +183,7 @@ struct mpc_dma_desc { dma_addr_t tcd_paddr; int error; struct list_head node; + int will_access_peripheral; }; struct mpc_dma_chan { > @@ -204,9 +201,13 @@ struct mpc_dma_chan { > struct list_head completed; > struct mpc_dma_tcd *tcd; > dma_addr_t tcd_paddr; > + u32 tcd_nunits; > > /* Lock for this structure */ > spinlock_t lock; > + > + /* Channel's peripheral fifo address */ > + dma_addr_t per_paddr; > }; > > struct mpc_dma { We can't chain together descriptors of transfers which involve peripherals because each of such transfers needs hardware initiated start. @@ -253,10 +248,27 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan) struct mpc_dma_desc *first = NULL; struct mpc_dma_desc *prev = NULL; struct mpc_dma_desc *mdesc; + int staffed = 0; int cid = mchan->chan.chan_id; - /* Move all queued descriptors to active list */ - list_splice_tail_init(&mchan->queued, &mchan->active); + /* + * Mem-to-mem transfers can be chained + * together into one transaction. + * But each transfer which involves peripherals + * must be executed separately. + */ + while (!staffed) { + mdesc = list_first_entry(&mchan->queued, + struct mpc_dma_desc, node); + + if (!mdesc->will_access_peripheral) + list_move_tail(&mdesc->node, &mchan->active); + else { + staffed = 1; + if (list_empty(&mchan->active)) + list_move_tail(&mdesc->node, &mchan->active); + } + } /* Chain descriptors into one transaction */ list_for_each_entry(mdesc, &mchan->active, node) { > @@ -270,7 +271,12 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan) > > prev->tcd->dlast_sga = mdesc->tcd_paddr; > prev->tcd->e_sg = 1; > - mdesc->tcd->start = 1; > + > + /* software start for mem-to-mem transfers */ > + if (mdma->is_mpc8308) > + mdesc->tcd->start = 1; > + else if (cid == MPC512x_DMACHAN_MDDRC) > + mdesc->tcd->start = 1; > > prev = mdesc; > } We don't need this change. Now only mem-to-mem transfers are chained: @@ -270,6 +282,8 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan) prev->tcd->dlast_sga = mdesc->tcd_paddr; prev->tcd->e_sg = 1; + + /* software start for mem-to-mem transfers */ mdesc->tcd->start = 1; prev = mdesc; > @@ -282,7 +288,17 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan) > > if (first != prev) > mdma->tcd[cid].e_sg = 1; > - out_8(&mdma->regs->dmassrt, cid); > + > + if (mdma->is_mpc8308) { > + /* MPC8308, no request lines, software initiated start */ > + out_8(&mdma->regs->dmassrt, cid); > + } else if (cid == MPC512x_DMACHAN_MDDRC) { This condition should be changed to: + } else if (!first->will_access_peripheral) { > + /* memory to memory transfer, software initiated start */ > + out_8(&mdma->regs->dmassrt, cid); > + } else { > + /* peripherals involved, use external request line */ > + out_8(&mdma->regs->dmaserq, cid); > + } > } > > /* Handle interrupt on one half of DMA controller (32 channels) */ Setting the flag: @@ -608,6 +632,7 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src, } mdesc->error = 0; + mdesc->will_access_peripheral = 0; tcd = mdesc->tcd; /* Prepare Transfer Control Descriptor for this transaction */ > @@ -655,6 +671,141 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src, > return &mdesc->desc; > } > > +static struct dma_async_tx_descriptor *mpc_dma_prep_slave_sg( > + struct dma_chan *chan, struct scatterlist *sgl, > + unsigned int sg_len, enum dma_transfer_direction direction, > + unsigned long flags, void *context) > +{ > + struct mpc_dma *mdma = dma_chan_to_mpc_dma(chan); > + struct mpc_dma_chan *mchan = dma_chan_to_mpc_dma_chan(chan); > + struct mpc_dma_desc *mdesc = NULL; Will be used when the spinlock is grabbed: + dma_addr_t per_paddr; + u32 tcd_nunits = 0; > + struct mpc_dma_tcd *tcd; > + unsigned long iflags; > + struct scatterlist *sg; > + size_t len; > + int iter, i; > + > + if (!list_empty(&mchan->active)) > + return NULL; > + > + /* currently there is no proper support for scatter/gather */ > + if (sg_len > 1) > + return NULL; > + > + for_each_sg(sgl, sg, sg_len, i) { > + spin_lock_irqsave(&mchan->lock, iflags); > + > + mdesc = list_first_entry(&mchan->free, struct mpc_dma_desc, > + node); > + if (!mdesc) { > + spin_unlock_irqrestore(&mchan->lock, iflags); > + /* try to free completed descriptors */ > + mpc_dma_process_completed(mdma); > + return NULL; > + } > + > + list_del(&mdesc->node); While spinlock is grabbed: + per_paddr = mchan->per_paddr; + tcd_nunits = mchan->tcd_nunits; > + > + spin_unlock_irqrestore(&mchan->lock, iflags); > + > + mdesc->error = 0; Setting the flag here: + mdesc->will_access_peripheral = 1; > + tcd = mdesc->tcd; > + > + /* Prepare Transfer Control Descriptor for this transaction */ > + memset(tcd, 0, sizeof(struct mpc_dma_tcd)); > + > + if (!IS_ALIGNED(sg_dma_address(sg), 4)) > + return NULL; > + > + if (direction == DMA_DEV_TO_MEM) { > + tcd->saddr = mchan->per_paddr; This line should be changed to: + tcd->saddr = per_paddr; > + tcd->daddr = sg_dma_address(sg); > + tcd->soff = 0; > + tcd->doff = 4; > + } else if (direction == DMA_MEM_TO_DEV) { > + tcd->saddr = sg_dma_address(sg); > + tcd->daddr = mchan->per_paddr; Ditto: + tcd->daddr = per_paddr; > + tcd->soff = 4; > + tcd->doff = 0; > + } else { > + return NULL; > + } > + tcd->ssize = MPC_DMA_TSIZE_4; > + tcd->dsize = MPC_DMA_TSIZE_4; > + > + len = sg_dma_len(sg); > + > + if (mchan->tcd_nunits) > + tcd->nbytes = mchan->tcd_nunits * 4; > + else > + tcd->nbytes = 64; These 4 lines should be changed to: + if (tcd_nunits) + tcd->nbytes = tcd_nunits * 4; + else + return NULL; The last line means that client kernel modules must set src_maxburst and dst_maxburst fields of dma_slave_config (dmaengine.h). > + > + if (!IS_ALIGNED(len, tcd->nbytes)) > + return NULL; > + > + iter = len / tcd->nbytes; > + if (iter > ((1 << 15) - 1)) { /* maximum biter */ > + return NULL; /* len is too big */ > + } else { > + /* citer_linkch contains the high bits of iter */ > + tcd->biter = iter & 0x1ff; > + tcd->biter_linkch = iter >> 9; > + tcd->citer = tcd->biter; > + tcd->citer_linkch = tcd->biter_linkch; > + } > + > + tcd->e_sg = 0; > + tcd->d_req = 1; > + > + /* Place descriptor in prepared list */ > + spin_lock_irqsave(&mchan->lock, iflags); > + list_add_tail(&mdesc->node, &mchan->prepared); > + spin_unlock_irqrestore(&mchan->lock, iflags); > + } > + > + /* Return the last descriptor */ Offer to remove this comment. > + return &mdesc->desc; > +} > + > +static int mpc_dma_device_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd, > + unsigned long arg) > +{ > + struct mpc_dma_chan *mchan; > + struct mpc_dma *mdma; > + struct dma_slave_config *cfg; Locking: + unsigned long flags; > + > + mchan = dma_chan_to_mpc_dma_chan(chan); > + switch (cmd) { > + case DMA_TERMINATE_ALL: > + /* disable channel requests */ > + mdma = dma_chan_to_mpc_dma(chan); + spin_lock_irqsave(&mchan->lock, flags); > + out_8(&mdma->regs->dmacerq, chan->chan_id); > + list_splice_tail_init(&mchan->prepared, &mchan->free); > + list_splice_tail_init(&mchan->queued, &mchan->free); > + list_splice_tail_init(&mchan->active, &mchan->free); + spin_unlock_irqrestore(&mchan->lock, flags); > + return 0; > + case DMA_SLAVE_CONFIG: > + cfg = (void *)arg; > + if (cfg->src_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES && > + cfg->dst_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES) > + return -EINVAL; + spin_lock_irqsave(&mchan->lock, flags); > + > + if (cfg->direction == DMA_DEV_TO_MEM) { > + mchan->per_paddr = cfg->src_addr; > + mchan->tcd_nunits = cfg->src_maxburst; > + } else { > + mchan->per_paddr = cfg->dst_addr; > + mchan->tcd_nunits = cfg->dst_maxburst; > + } + spin_unlock_irqrestore(&mchan->lock, flags); > + > + return 0; > + default: > + return -ENOSYS; > + } > + > + return -EINVAL; > +} > + > static int mpc_dma_probe(struct platform_device *op) > { > struct device_node *dn = op->dev.of_node; > @@ -739,9 +890,12 @@ static int mpc_dma_probe(struct platform_device *op) > dma->device_issue_pending = mpc_dma_issue_pending; > dma->device_tx_status = mpc_dma_tx_status; > dma->device_prep_dma_memcpy = mpc_dma_prep_memcpy; > + dma->device_prep_slave_sg = mpc_dma_prep_slave_sg; > + dma->device_control = mpc_dma_device_control; > > INIT_LIST_HEAD(&dma->channels); > dma_cap_set(DMA_MEMCPY, dma->cap_mask); > + dma_cap_set(DMA_SLAVE, dma->cap_mask); > > for (i = 0; i < dma->chancnt; i++) { > mchan = &mdma->channels[i]; > -- > 1.7.10.4 > Best regards, Alexander. ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH RFC v2 2/5] dma: mpc512x: add support for peripheral transfers @ 2013-08-12 13:37 ` Alexander Popov 0 siblings, 0 replies; 45+ messages in thread From: Alexander Popov @ 2013-08-12 13:37 UTC (permalink / raw) To: Gerhard Sittig Cc: Lars-Peter Clausen, Arnd Bergmann, Vinod Koul, linux-kernel, Dan Williams, Anatolij Gustschin, linuxppc-dev Hello everyone! Changes offered in this letter: - Adding a flag "will_access_peripheral" to DMA transfer descriptor according recommendations of Gerhard Sittig. This flag is set in mpc_dma_prep_memcpy() and mpc_dma_prep_slave_sg() and is evaluated in mpc_dma_execute(). - Adding locking and removing default nbytes value according recommendations of Lars-Peter Clausen. I tested these changes on MPC5125 with SCLPC driver (transfers between dev and mem work fine) and dmatest module (all 64 DMA channels can perform mem-to-mem transfers which can be chained in one DMA transaction). 2013/7/14 Gerhard Sittig <gsi@denx.de>: > From: Alexander Popov <a13xp0p0v88@gmail.com> > > introduce support for slave s/g transfer preparation and the associated > device control callback in the MPC512x DMA controller driver, which adds > support for data transfers between memory and peripheral I/O to the > previously supported mem-to-mem transfers > > refuse to prepare chunked transfers (transfers with more than one part) > as long as proper support for scatter/gather is lacking > > keep MPC8308 operational by always starting transfers from software, > this SoC appears to not have request lines for flow control when > peripherals are involved in transfers > > [ introduction of slave s/g preparation and device control ] > Signed-off-by: Alexander Popov <a13xp0p0v88@gmail.com> > [ execute() start condition, mpc8308 compat, terminate all, s/g length check, reworded commit msg ] > Signed-off-by: Gerhard Sittig <gsi@denx.de> > --- > drivers/dma/mpc512x_dma.c | 168 +++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 161 insertions(+), 7 deletions(-) > > diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c > index 4e03f1e..55e6a87 100644 > --- a/drivers/dma/mpc512x_dma.c > +++ b/drivers/dma/mpc512x_dma.c > @@ -2,6 +2,7 @@ > * Copyright (C) Freescale Semicondutor, Inc. 2007, 2008. > * Copyright (C) Semihalf 2009 > * Copyright (C) Ilya Yanok, Emcraft Systems 2010 > + * Copyright (C) Alexander Popov, Promcontroller 2013 > * > * Written by Piotr Ziecik <kosmo@semihalf.com>. Hardware description > * (defines, structures and comments) was taken from MPC5121 DMA driver > @@ -28,11 +29,6 @@ > * file called COPYING. > */ > > -/* > - * This is initial version of MPC5121 DMA driver. Only memory to memory > - * transfers are supported (tested using dmatest module). > - */ > - > #include <linux/module.h> > #include <linux/dmaengine.h> > #include <linux/dma-mapping.h> > @@ -63,6 +59,7 @@ enum mpc8308_dmachan_id_t { > MPC8308_DMACHAN_MAX = 16, > }; > enum mpc512x_dmachan_id_t { > + MPC512x_DMACHAN_MDDRC = 32, > MPC512x_DMACHAN_MAX = 64, > }; > #define MPC_DMA_CHANNELS 64 Adding the flag: @@ -193,6 +183,7 @@ struct mpc_dma_desc { dma_addr_t tcd_paddr; int error; struct list_head node; + int will_access_peripheral; }; struct mpc_dma_chan { > @@ -204,9 +201,13 @@ struct mpc_dma_chan { > struct list_head completed; > struct mpc_dma_tcd *tcd; > dma_addr_t tcd_paddr; > + u32 tcd_nunits; > > /* Lock for this structure */ > spinlock_t lock; > + > + /* Channel's peripheral fifo address */ > + dma_addr_t per_paddr; > }; > > struct mpc_dma { We can't chain together descriptors of transfers which involve peripherals because each of such transfers needs hardware initiated start. @@ -253,10 +248,27 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan) struct mpc_dma_desc *first = NULL; struct mpc_dma_desc *prev = NULL; struct mpc_dma_desc *mdesc; + int staffed = 0; int cid = mchan->chan.chan_id; - /* Move all queued descriptors to active list */ - list_splice_tail_init(&mchan->queued, &mchan->active); + /* + * Mem-to-mem transfers can be chained + * together into one transaction. + * But each transfer which involves peripherals + * must be executed separately. + */ + while (!staffed) { + mdesc = list_first_entry(&mchan->queued, + struct mpc_dma_desc, node); + + if (!mdesc->will_access_peripheral) + list_move_tail(&mdesc->node, &mchan->active); + else { + staffed = 1; + if (list_empty(&mchan->active)) + list_move_tail(&mdesc->node, &mchan->active); + } + } /* Chain descriptors into one transaction */ list_for_each_entry(mdesc, &mchan->active, node) { > @@ -270,7 +271,12 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan) > > prev->tcd->dlast_sga = mdesc->tcd_paddr; > prev->tcd->e_sg = 1; > - mdesc->tcd->start = 1; > + > + /* software start for mem-to-mem transfers */ > + if (mdma->is_mpc8308) > + mdesc->tcd->start = 1; > + else if (cid == MPC512x_DMACHAN_MDDRC) > + mdesc->tcd->start = 1; > > prev = mdesc; > } We don't need this change. Now only mem-to-mem transfers are chained: @@ -270,6 +282,8 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan) prev->tcd->dlast_sga = mdesc->tcd_paddr; prev->tcd->e_sg = 1; + + /* software start for mem-to-mem transfers */ mdesc->tcd->start = 1; prev = mdesc; > @@ -282,7 +288,17 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan) > > if (first != prev) > mdma->tcd[cid].e_sg = 1; > - out_8(&mdma->regs->dmassrt, cid); > + > + if (mdma->is_mpc8308) { > + /* MPC8308, no request lines, software initiated start */ > + out_8(&mdma->regs->dmassrt, cid); > + } else if (cid == MPC512x_DMACHAN_MDDRC) { This condition should be changed to: + } else if (!first->will_access_peripheral) { > + /* memory to memory transfer, software initiated start */ > + out_8(&mdma->regs->dmassrt, cid); > + } else { > + /* peripherals involved, use external request line */ > + out_8(&mdma->regs->dmaserq, cid); > + } > } > > /* Handle interrupt on one half of DMA controller (32 channels) */ Setting the flag: @@ -608,6 +632,7 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src, } mdesc->error = 0; + mdesc->will_access_peripheral = 0; tcd = mdesc->tcd; /* Prepare Transfer Control Descriptor for this transaction */ > @@ -655,6 +671,141 @@ mpc_dma_prep_memcpy(struct dma_chan *chan, dma_addr_t dst, dma_addr_t src, > return &mdesc->desc; > } > > +static struct dma_async_tx_descriptor *mpc_dma_prep_slave_sg( > + struct dma_chan *chan, struct scatterlist *sgl, > + unsigned int sg_len, enum dma_transfer_direction direction, > + unsigned long flags, void *context) > +{ > + struct mpc_dma *mdma = dma_chan_to_mpc_dma(chan); > + struct mpc_dma_chan *mchan = dma_chan_to_mpc_dma_chan(chan); > + struct mpc_dma_desc *mdesc = NULL; Will be used when the spinlock is grabbed: + dma_addr_t per_paddr; + u32 tcd_nunits = 0; > + struct mpc_dma_tcd *tcd; > + unsigned long iflags; > + struct scatterlist *sg; > + size_t len; > + int iter, i; > + > + if (!list_empty(&mchan->active)) > + return NULL; > + > + /* currently there is no proper support for scatter/gather */ > + if (sg_len > 1) > + return NULL; > + > + for_each_sg(sgl, sg, sg_len, i) { > + spin_lock_irqsave(&mchan->lock, iflags); > + > + mdesc = list_first_entry(&mchan->free, struct mpc_dma_desc, > + node); > + if (!mdesc) { > + spin_unlock_irqrestore(&mchan->lock, iflags); > + /* try to free completed descriptors */ > + mpc_dma_process_completed(mdma); > + return NULL; > + } > + > + list_del(&mdesc->node); While spinlock is grabbed: + per_paddr = mchan->per_paddr; + tcd_nunits = mchan->tcd_nunits; > + > + spin_unlock_irqrestore(&mchan->lock, iflags); > + > + mdesc->error = 0; Setting the flag here: + mdesc->will_access_peripheral = 1; > + tcd = mdesc->tcd; > + > + /* Prepare Transfer Control Descriptor for this transaction */ > + memset(tcd, 0, sizeof(struct mpc_dma_tcd)); > + > + if (!IS_ALIGNED(sg_dma_address(sg), 4)) > + return NULL; > + > + if (direction == DMA_DEV_TO_MEM) { > + tcd->saddr = mchan->per_paddr; This line should be changed to: + tcd->saddr = per_paddr; > + tcd->daddr = sg_dma_address(sg); > + tcd->soff = 0; > + tcd->doff = 4; > + } else if (direction == DMA_MEM_TO_DEV) { > + tcd->saddr = sg_dma_address(sg); > + tcd->daddr = mchan->per_paddr; Ditto: + tcd->daddr = per_paddr; > + tcd->soff = 4; > + tcd->doff = 0; > + } else { > + return NULL; > + } > + tcd->ssize = MPC_DMA_TSIZE_4; > + tcd->dsize = MPC_DMA_TSIZE_4; > + > + len = sg_dma_len(sg); > + > + if (mchan->tcd_nunits) > + tcd->nbytes = mchan->tcd_nunits * 4; > + else > + tcd->nbytes = 64; These 4 lines should be changed to: + if (tcd_nunits) + tcd->nbytes = tcd_nunits * 4; + else + return NULL; The last line means that client kernel modules must set src_maxburst and dst_maxburst fields of dma_slave_config (dmaengine.h). > + > + if (!IS_ALIGNED(len, tcd->nbytes)) > + return NULL; > + > + iter = len / tcd->nbytes; > + if (iter > ((1 << 15) - 1)) { /* maximum biter */ > + return NULL; /* len is too big */ > + } else { > + /* citer_linkch contains the high bits of iter */ > + tcd->biter = iter & 0x1ff; > + tcd->biter_linkch = iter >> 9; > + tcd->citer = tcd->biter; > + tcd->citer_linkch = tcd->biter_linkch; > + } > + > + tcd->e_sg = 0; > + tcd->d_req = 1; > + > + /* Place descriptor in prepared list */ > + spin_lock_irqsave(&mchan->lock, iflags); > + list_add_tail(&mdesc->node, &mchan->prepared); > + spin_unlock_irqrestore(&mchan->lock, iflags); > + } > + > + /* Return the last descriptor */ Offer to remove this comment. > + return &mdesc->desc; > +} > + > +static int mpc_dma_device_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd, > + unsigned long arg) > +{ > + struct mpc_dma_chan *mchan; > + struct mpc_dma *mdma; > + struct dma_slave_config *cfg; Locking: + unsigned long flags; > + > + mchan = dma_chan_to_mpc_dma_chan(chan); > + switch (cmd) { > + case DMA_TERMINATE_ALL: > + /* disable channel requests */ > + mdma = dma_chan_to_mpc_dma(chan); + spin_lock_irqsave(&mchan->lock, flags); > + out_8(&mdma->regs->dmacerq, chan->chan_id); > + list_splice_tail_init(&mchan->prepared, &mchan->free); > + list_splice_tail_init(&mchan->queued, &mchan->free); > + list_splice_tail_init(&mchan->active, &mchan->free); + spin_unlock_irqrestore(&mchan->lock, flags); > + return 0; > + case DMA_SLAVE_CONFIG: > + cfg = (void *)arg; > + if (cfg->src_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES && > + cfg->dst_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES) > + return -EINVAL; + spin_lock_irqsave(&mchan->lock, flags); > + > + if (cfg->direction == DMA_DEV_TO_MEM) { > + mchan->per_paddr = cfg->src_addr; > + mchan->tcd_nunits = cfg->src_maxburst; > + } else { > + mchan->per_paddr = cfg->dst_addr; > + mchan->tcd_nunits = cfg->dst_maxburst; > + } + spin_unlock_irqrestore(&mchan->lock, flags); > + > + return 0; > + default: > + return -ENOSYS; > + } > + > + return -EINVAL; > +} > + > static int mpc_dma_probe(struct platform_device *op) > { > struct device_node *dn = op->dev.of_node; > @@ -739,9 +890,12 @@ static int mpc_dma_probe(struct platform_device *op) > dma->device_issue_pending = mpc_dma_issue_pending; > dma->device_tx_status = mpc_dma_tx_status; > dma->device_prep_dma_memcpy = mpc_dma_prep_memcpy; > + dma->device_prep_slave_sg = mpc_dma_prep_slave_sg; > + dma->device_control = mpc_dma_device_control; > > INIT_LIST_HEAD(&dma->channels); > dma_cap_set(DMA_MEMCPY, dma->cap_mask); > + dma_cap_set(DMA_SLAVE, dma->cap_mask); > > for (i = 0; i < dma->chancnt; i++) { > mchan = &mdma->channels[i]; > -- > 1.7.10.4 > Best regards, Alexander. ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH RFC v2 3/5] dma: of: Add common xlate function for matching by channel id 2013-07-14 12:01 ` Gerhard Sittig ` (2 preceding siblings ...) (?) @ 2013-07-14 12:01 ` Gerhard Sittig 2013-10-03 14:05 ` Alexander Popov -1 siblings, 1 reply; 45+ messages in thread From: Gerhard Sittig @ 2013-07-14 12:01 UTC (permalink / raw) To: linuxppc-dev, devicetree-discuss, Alexander Popov Cc: Lars-Peter Clausen, Arnd Bergmann, Vinod Koul, Gerhard Sittig, Dan Williams, Anatolij Gustschin From: Lars-Peter Clausen <lars@metafoo.de> This patch adds a new common OF dma xlate callback function which will match a channel by it's id. The binding expects one integer argument which it will use to lookup the channel by the id. Unlike of_dma_simple_xlate this function is able to handle a system with multiple DMA controllers. When registering the of dma provider with of_dma_controller_register a pointer to the dma_device struct which is associated with the dt node needs to passed as the data parameter. The filter function will use this pointer to match only channels which belong to the specified DMA controller. Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> Signed-off-by: Gerhard Sittig <gsi@denx.de> --- drivers/dma/of-dma.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ include/linux/of_dma.h | 4 ++++ 2 files changed, 51 insertions(+) diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c index 7aa0864..d5d528e 100644 --- a/drivers/dma/of-dma.c +++ b/drivers/dma/of-dma.c @@ -229,3 +229,50 @@ struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec, &dma_spec->args[0]); } EXPORT_SYMBOL_GPL(of_dma_simple_xlate); + +struct of_dma_filter_by_chan_id_args { + struct dma_device *dev; + unsigned int chan_id; +}; + +static bool of_dma_filter_by_chan_id(struct dma_chan *chan, void *params) +{ + struct of_dma_filter_by_chan_id_args *args = params; + + return chan->device == args->dev && chan->chan_id == args->chan_id; +} + +/** + * of_dma_xlate_by_chan_id - Translate dt property to DMA channel by channel id + * @dma_spec: pointer to DMA specifier as found in the device tree + * @of_dma: pointer to DMA controller data + * + * This function can be used as the of xlate callback for DMA driver which wants + * to match the channel based on the channel id. When using this xlate function + * the #dma-cells propety of the DMA controller dt node needs to be set to 1. + * The data parameter of of_dma_controller_register must be a pointer to the + * dma_device struct the function should match upon. + * + * Returns pointer to appropriate dma channel on success or NULL on error. + */ +struct dma_chan *of_dma_xlate_by_chan_id(struct of_phandle_args *dma_spec, + struct of_dma *ofdma) +{ + struct of_dma_filter_by_chan_id_args args; + dma_cap_mask_t cap; + + args.dev = ofdma->of_dma_data; + if (!args.dev) + return NULL; + + if (dma_spec->args_count != 1) + return NULL; + + dma_cap_zero(cap); + dma_cap_set(DMA_SLAVE, cap); + + args.chan_id = dma_spec->args[0]; + + return dma_request_channel(cap, of_dma_filter_by_chan_id, &args); +} +EXPORT_SYMBOL_GPL(of_dma_xlate_by_chan_id); diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h index 364dda7..b7cf614 100644 --- a/include/linux/of_dma.h +++ b/include/linux/of_dma.h @@ -42,6 +42,8 @@ extern struct dma_chan *of_dma_request_slave_channel(struct device_node *np, const char *name); extern struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec, struct of_dma *ofdma); +extern struct dma_chan *of_dma_xlate_by_chan_id(struct of_phandle_args *dma_spec, + struct of_dma *ofdma); #else static inline int of_dma_controller_register(struct device_node *np, struct dma_chan *(*of_dma_xlate) @@ -67,6 +69,8 @@ static inline struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_s return NULL; } +#define of_dma_xlate_by_chan_id NULL + #endif #endif /* __LINUX_OF_DMA_H */ -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH RFC v2 3/5] dma: of: Add common xlate function for matching by channel id 2013-07-14 12:01 ` [PATCH RFC v2 3/5] dma: of: Add common xlate function for matching by channel id Gerhard Sittig @ 2013-10-03 14:05 ` Alexander Popov 0 siblings, 0 replies; 45+ messages in thread From: Alexander Popov @ 2013-10-03 14:05 UTC (permalink / raw) To: Gerhard Sittig Cc: Lars-Peter Clausen, Arnd Bergmann, Vinod Koul, devicetree-discuss, Alexander Popov, Dan Williams, Anatolij Gustschin, linuxppc-dev From: Lars-Peter Clausen <lars@metafoo.de> This patch adds a new common OF dma xlate callback function which will match a channel by it's id. The binding expects one integer argument which it will use to lookup the channel by the id. Unlike of_dma_simple_xlate this function is able to handle a system with multiple DMA controllers. When registering the of dma provider with of_dma_controller_register a pointer to the dma_device struct which is associated with the dt node needs to passed as the data parameter. The filter function will use this pointer to match only channels which belong to the specified DMA controller. Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> Signed-off-by: Gerhard Sittig <gsi@denx.de> --- drivers/dma/of-dma.c | 47 ++++++++++++++++++++++++++++++ +++++++++++++++++ include/linux/of_dma.h | 4 ++++ 2 files changed, 51 insertions(+) diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c index 7aa0864..d5d528e 100644 --- a/drivers/dma/of-dma.c +++ b/drivers/dma/of-dma.c @@ -229,3 +229,50 @@ struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec, &dma_spec->args[0]); } EXPORT_SYMBOL_GPL(of_dma_simple_xlate); + +struct of_dma_filter_by_chan_id_args { + struct dma_device *dev; + unsigned int chan_id; +}; + +static bool of_dma_filter_by_chan_id(struct dma_chan *chan, void *params) +{ + struct of_dma_filter_by_chan_id_args *args = params; + + return chan->device == args->dev && chan->chan_id == args->chan_id; +} + +/** + * of_dma_xlate_by_chan_id - Translate dt property to DMA channel by channel id + * @dma_spec: pointer to DMA specifier as found in the device tree + * @of_dma: pointer to DMA controller data + * + * This function can be used as the of xlate callback for DMA driver which wants + * to match the channel based on the channel id. When using this xlate function + * the #dma-cells propety of the DMA controller dt node needs to be set to 1. + * The data parameter of of_dma_controller_register must be a pointer to the + * dma_device struct the function should match upon. + * + * Returns pointer to appropriate dma channel on success or NULL on error. + */ +struct dma_chan *of_dma_xlate_by_chan_id(struct of_phandle_args *dma_spec, + struct of_dma *ofdma) +{ + struct of_dma_filter_by_chan_id_args args; + dma_cap_mask_t cap; + + args.dev = ofdma->of_dma_data; + if (!args.dev) + return NULL; + + if (dma_spec->args_count != 1) + return NULL; + + dma_cap_zero(cap); + dma_cap_set(DMA_SLAVE, cap); + + args.chan_id = dma_spec->args[0]; + + return dma_request_channel(cap, of_dma_filter_by_chan_id, &args); +} +EXPORT_SYMBOL_GPL(of_dma_xlate_by_chan_id); diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h index 364dda7..b7cf614 100644 --- a/include/linux/of_dma.h +++ b/include/linux/of_dma.h @@ -42,6 +42,8 @@ extern struct dma_chan *of_dma_request_slave_channel(struct device_node *np, const char *name); extern struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec, struct of_dma *ofdma); +extern struct dma_chan *of_dma_xlate_by_chan_id(struct of_phandle_args *dma_spec, + struct of_dma *ofdma); #else static inline int of_dma_controller_register(struct device_node *np, struct dma_chan *(*of_dma_xlate) @@ -67,6 +69,8 @@ static inline struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_s return NULL; } +#define of_dma_xlate_by_chan_id NULL + #endif #endif /* __LINUX_OF_DMA_H */ -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH RFC v2 4/5] dma: mpc512x: register for device tree channel lookup 2013-07-14 12:01 ` Gerhard Sittig ` (3 preceding siblings ...) (?) @ 2013-07-14 12:02 ` Gerhard Sittig 2013-10-03 14:06 ` Alexander Popov -1 siblings, 1 reply; 45+ messages in thread From: Gerhard Sittig @ 2013-07-14 12:02 UTC (permalink / raw) To: linuxppc-dev, devicetree-discuss, Alexander Popov Cc: Lars-Peter Clausen, Arnd Bergmann, Vinod Koul, Gerhard Sittig, Dan Williams, Anatolij Gustschin register the controller for device tree based lookup of DMA channels (non-fatal for backwards compatibility with older device trees), provide the '#dma-cells' property in the shared mpc5121.dtsi file, and introduce a bindings document for the MPC512x DMA controller Signed-off-by: Gerhard Sittig <gsi@denx.de> --- .../devicetree/bindings/dma/mpc512x-dma.txt | 55 ++++++++++++++++++++ arch/powerpc/boot/dts/mpc5121.dtsi | 1 + drivers/dma/mpc512x_dma.c | 21 ++++++-- 3 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 Documentation/devicetree/bindings/dma/mpc512x-dma.txt diff --git a/Documentation/devicetree/bindings/dma/mpc512x-dma.txt b/Documentation/devicetree/bindings/dma/mpc512x-dma.txt new file mode 100644 index 0000000..a4867d5 --- /dev/null +++ b/Documentation/devicetree/bindings/dma/mpc512x-dma.txt @@ -0,0 +1,55 @@ +* Freescale MPC512x DMA Controller + +The DMA controller in the Freescale MPC512x SoC can move blocks of +memory contents between memory and peripherals or memory to memory. + +Refer to the "Generic DMA Controller and DMA request bindings" description +in the dma.txt file for a more detailled discussion of the binding. The +MPC512x DMA engine binding follows the common scheme, but doesn't provide +support for the optional channels and requests counters (those values are +derived from the detected hardware features) and has a fixed client +specifier length of 1 integer cell (the value is the DMA channel, since +the DMA controller uses a fixed assignment of request lines per channel). + + +DMA controller node properties: + +Required properties: +- compatible: should be "fsl,mpc5121-dma" +- reg: address and size of the DMA controller's register set +- interrupts: interrupt spec for the DMA controller + +Optional properties: +- #dma-cells: must be <1>, describes the number of integer cells + needed to specify the 'dmas' property in client nodes, + strongly recommended since common client helper code + uses this property + +Example: + + dma0: dma@14000 { + compatible = "fsl,mpc5121-dma"; + reg = <0x14000 0x1800>; + interrupts = <65 0x8>; + #dma-cells = <1>; + }; + + +Client node properties: + +Required properties: +- dmas: list of DMA specifiers, consisting each of a handle + for the DMA controller and integer cells to specify + the channel used within the DMA controller +- dma-names: list of identifier strings for the DMA specifiers, + client device driver code uses these strings to + have DMA channels looked up at the controller + +Example: + + sdhc@1500 { + compatible = "fsl,mpc5121-sdhc"; + /* ... */ + dmas = <&dma0 30>; + dma-names = "rx-tx"; + }; diff --git a/arch/powerpc/boot/dts/mpc5121.dtsi b/arch/powerpc/boot/dts/mpc5121.dtsi index bd14c00..eb0058f 100644 --- a/arch/powerpc/boot/dts/mpc5121.dtsi +++ b/arch/powerpc/boot/dts/mpc5121.dtsi @@ -390,6 +390,7 @@ compatible = "fsl,mpc5121-dma"; reg = <0x14000 0x1800>; interrupts = <65 0x8>; + #dma-cells = <1>; }; }; diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c index 55e6a87..84fd07b 100644 --- a/drivers/dma/mpc512x_dma.c +++ b/drivers/dma/mpc512x_dma.c @@ -36,6 +36,7 @@ #include <linux/io.h> #include <linux/slab.h> #include <linux/of_device.h> +#include <linux/of_dma.h> #include <linux/of_platform.h> #include <linux/random.h> @@ -959,11 +960,23 @@ static int mpc_dma_probe(struct platform_device *op) /* Register DMA engine */ dev_set_drvdata(dev, mdma); retval = dma_async_device_register(dma); - if (retval) { - devm_free_irq(dev, mdma->irq, mdma); - irq_dispose_mapping(mdma->irq); + if (retval) + goto out_irq; + + /* register with OF helpers for DMA lookups (nonfatal) */ + if (dev->of_node) { + retval = of_dma_controller_register(dev->of_node, + of_dma_xlate_by_chan_id, + mdma); + if (retval) + dev_warn(dev, "could not register for OF lookup\n"); } + return 0; + +out_irq: + devm_free_irq(dev, mdma->irq, mdma); + irq_dispose_mapping(mdma->irq); return retval; } @@ -972,6 +985,8 @@ static int mpc_dma_remove(struct platform_device *op) struct device *dev = &op->dev; struct mpc_dma *mdma = dev_get_drvdata(dev); + if (dev->of_node) + of_dma_controller_free(dev->of_node); dma_async_device_unregister(&mdma->dma); devm_free_irq(dev, mdma->irq, mdma); irq_dispose_mapping(mdma->irq); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH RFC v2 4/5] dma: mpc512x: register for device tree channel lookup 2013-07-14 12:02 ` [PATCH RFC v2 4/5] dma: mpc512x: register for device tree channel lookup Gerhard Sittig @ 2013-10-03 14:06 ` Alexander Popov 0 siblings, 0 replies; 45+ messages in thread From: Alexander Popov @ 2013-10-03 14:06 UTC (permalink / raw) To: Gerhard Sittig Cc: Lars-Peter Clausen, Arnd Bergmann, Vinod Koul, devicetree-discuss, Alexander Popov, Dan Williams, Anatolij Gustschin, linuxppc-dev From: Gerhard Sittig <gsi@denx.de> register the controller for device tree based lookup of DMA channels (non-fatal for backwards compatibility with older device trees), provide the '#dma-cells' property in the shared mpc5121.dtsi file, and introduce a bindings document for the MPC512x DMA controller Signed-off-by: Gerhard Sittig <gsi@denx.de> --- .../devicetree/bindings/dma/ mpc512x-dma.txt | 55 ++++++++++++++++++++ arch/powerpc/boot/dts/mpc5121.dtsi | 1 + drivers/dma/mpc512x_dma.c | 21 ++++++-- 3 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 Documentation/devicetree/bindings/dma/mpc512x-dma.txt diff --git a/Documentation/devicetree/bindings/dma/mpc512x-dma.txt b/Documentation/devicetree/bindings/dma/mpc512x-dma.txt new file mode 100644 index 0000000..a4867d5 --- /dev/null +++ b/Documentation/devicetree/bindings/dma/mpc512x-dma.txt @@ -0,0 +1,55 @@ +* Freescale MPC512x DMA Controller + +The DMA controller in the Freescale MPC512x SoC can move blocks of +memory contents between memory and peripherals or memory to memory. + +Refer to the "Generic DMA Controller and DMA request bindings" description +in the dma.txt file for a more detailled discussion of the binding. The +MPC512x DMA engine binding follows the common scheme, but doesn't provide +support for the optional channels and requests counters (those values are +derived from the detected hardware features) and has a fixed client +specifier length of 1 integer cell (the value is the DMA channel, since +the DMA controller uses a fixed assignment of request lines per channel). + + +DMA controller node properties: + +Required properties: +- compatible: should be "fsl,mpc5121-dma" +- reg: address and size of the DMA controller's register set +- interrupts: interrupt spec for the DMA controller + +Optional properties: +- #dma-cells: must be <1>, describes the number of integer cells + needed to specify the 'dmas' property in client nodes, + strongly recommended since common client helper code + uses this property + +Example: + + dma0: dma@14000 { + compatible = "fsl,mpc5121-dma"; + reg = <0x14000 0x1800>; + interrupts = <65 0x8>; + #dma-cells = <1>; + }; + + +Client node properties: + +Required properties: +- dmas: list of DMA specifiers, consisting each of a handle + for the DMA controller and integer cells to specify + the channel used within the DMA controller +- dma-names: list of identifier strings for the DMA specifiers, + client device driver code uses these strings to + have DMA channels looked up at the controller + +Example: + + sdhc@1500 { + compatible = "fsl,mpc5121-sdhc"; + /* ... */ + dmas = <&dma0 30>; + dma-names = "rx-tx"; + }; diff --git a/arch/powerpc/boot/dts/mpc5121.dtsi b/arch/powerpc/boot/dts/mpc5121.dtsi index 384e692..dae99b7 100644 --- a/arch/powerpc/boot/dts/mpc5121.dtsi +++ b/arch/powerpc/boot/dts/mpc5121.dtsi @@ -394,6 +394,7 @@ compatible = "fsl,mpc5121-dma"; reg = <0x14000 0x1800>; interrupts = <65 0x8>; + #dma-cells = <1>; }; }; diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c index 8f6d545..3d79e3a 100644 --- a/drivers/dma/mpc512x_dma.c +++ b/drivers/dma/mpc512x_dma.c @@ -38,6 +38,7 @@ #include <linux/io.h> #include <linux/slab.h> #include <linux/of_device.h> +#include <linux/of_dma.h> #include <linux/of_platform.h> #include <linux/random.h> @@ -939,11 +940,23 @@ static int mpc_dma_probe(struct platform_device *op) /* Register DMA engine */ dev_set_drvdata(dev, mdma); retval = dma_async_device_register(dma); - if (retval) { - devm_free_irq(dev, mdma->irq, mdma); - irq_dispose_mapping(mdma->irq); + if (retval) + goto out_irq; + + /* register with OF helpers for DMA lookups (nonfatal) */ + if (dev->of_node) { + retval = of_dma_controller_register(dev->of_node, + of_dma_xlate_by_chan_id, + mdma); + if (retval) + dev_warn(dev, "could not register for OF lookup\n"); } + return 0; + +out_irq: + devm_free_irq(dev, mdma->irq, mdma); + irq_dispose_mapping(mdma->irq); return retval; } @@ -952,6 +965,8 @@ static int mpc_dma_remove(struct platform_device *op) struct device *dev = &op->dev; struct mpc_dma *mdma = dev_get_drvdata(dev); + if (dev->of_node) + of_dma_controller_free(dev->of_node); dma_async_device_unregister(&mdma->dma); devm_free_irq(dev, mdma->irq, mdma); irq_dispose_mapping(mdma->irq); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH RFC v2 5/5] HACK mmc: mxcmmc: enable clocks for the MPC512x 2013-07-14 12:01 ` Gerhard Sittig ` (4 preceding siblings ...) (?) @ 2013-07-14 12:02 ` Gerhard Sittig 2013-10-03 14:06 ` Alexander Popov -1 siblings, 1 reply; 45+ messages in thread From: Gerhard Sittig @ 2013-07-14 12:02 UTC (permalink / raw) To: linuxppc-dev, devicetree-discuss, Alexander Popov Cc: Lars-Peter Clausen, Arnd Bergmann, Vinod Koul, Gerhard Sittig, Dan Williams, Anatolij Gustschin Q&D HACK to enable SD card support without correct COMMON_CLK support, best viewed with 'git diff -w -b', NOT acceptable for mainline (NAKed) Signed-off-by: Gerhard Sittig <gsi@denx.de> --- drivers/mmc/host/mxcmmc.c | 41 +++++++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c index d503635..b9d21cc 100644 --- a/drivers/mmc/host/mxcmmc.c +++ b/drivers/mmc/host/mxcmmc.c @@ -1121,20 +1121,29 @@ static int mxcmci_probe(struct platform_device *pdev) host->res = r; host->irq = irq; - host->clk_ipg = devm_clk_get(&pdev->dev, "ipg"); - if (IS_ERR(host->clk_ipg)) { - ret = PTR_ERR(host->clk_ipg); - goto out_iounmap; - } + if (!is_mpc512x_mmc(host)) { + host->clk_ipg = devm_clk_get(&pdev->dev, "ipg"); + if (IS_ERR(host->clk_ipg)) { + ret = PTR_ERR(host->clk_ipg); + goto out_iounmap; + } - host->clk_per = devm_clk_get(&pdev->dev, "per"); - if (IS_ERR(host->clk_per)) { - ret = PTR_ERR(host->clk_per); - goto out_iounmap; + host->clk_per = devm_clk_get(&pdev->dev, "per"); + if (IS_ERR(host->clk_per)) { + ret = PTR_ERR(host->clk_per); + goto out_iounmap; + } + } else { + host->clk_per = devm_clk_get(&pdev->dev, "sdhc_clk"); + if (IS_ERR(host->clk_per)) { + ret = PTR_ERR(host->clk_per); + goto out_iounmap; + } } clk_prepare_enable(host->clk_per); - clk_prepare_enable(host->clk_ipg); + if (host->clk_ipg) + clk_prepare_enable(host->clk_ipg); mxcmci_softreset(host); @@ -1204,7 +1213,8 @@ out_free_dma: dma_release_channel(host->dma); out_clk_put: clk_disable_unprepare(host->clk_per); - clk_disable_unprepare(host->clk_ipg); + if (host->clk_ipg) + clk_disable_unprepare(host->clk_ipg); out_iounmap: iounmap(host->base); out_free: @@ -1236,7 +1246,8 @@ static int mxcmci_remove(struct platform_device *pdev) dma_release_channel(host->dma); clk_disable_unprepare(host->clk_per); - clk_disable_unprepare(host->clk_ipg); + if (host->clk_ipg) + clk_disable_unprepare(host->clk_ipg); release_mem_region(host->res->start, resource_size(host->res)); @@ -1255,7 +1266,8 @@ static int mxcmci_suspend(struct device *dev) if (mmc) ret = mmc_suspend_host(mmc); clk_disable_unprepare(host->clk_per); - clk_disable_unprepare(host->clk_ipg); + if (host->clk_ipg) + clk_disable_unprepare(host->clk_ipg); return ret; } @@ -1267,7 +1279,8 @@ static int mxcmci_resume(struct device *dev) int ret = 0; clk_prepare_enable(host->clk_per); - clk_prepare_enable(host->clk_ipg); + if (host->clk_ipg) + clk_prepare_enable(host->clk_ipg); if (mmc) ret = mmc_resume_host(mmc); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH RFC v2 5/5] HACK mmc: mxcmmc: enable clocks for the MPC512x 2013-07-14 12:02 ` [PATCH RFC v2 5/5] HACK mmc: mxcmmc: enable clocks for the MPC512x Gerhard Sittig @ 2013-10-03 14:06 ` Alexander Popov 0 siblings, 0 replies; 45+ messages in thread From: Alexander Popov @ 2013-10-03 14:06 UTC (permalink / raw) To: Gerhard Sittig Cc: Lars-Peter Clausen, Arnd Bergmann, Vinod Koul, devicetree-discuss, Alexander Popov, Dan Williams, Anatolij Gustschin, linuxppc-dev From: Gerhard Sittig <gsi@denx.de> Q&D HACK to enable SD card support without correct COMMON_CLK support, best viewed with 'git diff -w -b', NOT acceptable for mainline (NAKed) Signed-off-by: Gerhard Sittig <gsi@denx.de> --- drivers/mmc/host/mxcmmc.c | 41 +++++++++++++++++++++++++++--- ----------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c index d503635..b9d21cc 100644 --- a/drivers/mmc/host/mxcmmc.c +++ b/drivers/mmc/host/mxcmmc.c @@ -1121,20 +1121,29 @@ static int mxcmci_probe(struct platform_device *pdev) host->res = r; host->irq = irq; - host->clk_ipg = devm_clk_get(&pdev->dev, "ipg"); - if (IS_ERR(host->clk_ipg)) { - ret = PTR_ERR(host->clk_ipg); - goto out_iounmap; - } + if (!is_mpc512x_mmc(host)) { + host->clk_ipg = devm_clk_get(&pdev->dev, "ipg"); + if (IS_ERR(host->clk_ipg)) { + ret = PTR_ERR(host->clk_ipg); + goto out_iounmap; + } - host->clk_per = devm_clk_get(&pdev->dev, "per"); - if (IS_ERR(host->clk_per)) { - ret = PTR_ERR(host->clk_per); - goto out_iounmap; + host->clk_per = devm_clk_get(&pdev->dev, "per"); + if (IS_ERR(host->clk_per)) { + ret = PTR_ERR(host->clk_per); + goto out_iounmap; + } + } else { + host->clk_per = devm_clk_get(&pdev->dev, "sdhc_clk"); + if (IS_ERR(host->clk_per)) { + ret = PTR_ERR(host->clk_per); + goto out_iounmap; + } } clk_prepare_enable(host->clk_per); - clk_prepare_enable(host->clk_ipg); + if (host->clk_ipg) + clk_prepare_enable(host->clk_ipg); mxcmci_softreset(host); @@ -1204,7 +1213,8 @@ out_free_dma: dma_release_channel(host->dma); out_clk_put: clk_disable_unprepare(host->clk_per); - clk_disable_unprepare(host->clk_ipg); + if (host->clk_ipg) + clk_disable_unprepare(host->clk_ipg); out_iounmap: iounmap(host->base); out_free: @@ -1236,7 +1246,8 @@ static int mxcmci_remove(struct platform_device *pdev) dma_release_channel(host->dma); clk_disable_unprepare(host->clk_per); - clk_disable_unprepare(host->clk_ipg); + if (host->clk_ipg) + clk_disable_unprepare(host->clk_ipg); release_mem_region(host->res->start, resource_size(host->res)); @@ -1255,7 +1266,8 @@ static int mxcmci_suspend(struct device *dev) if (mmc) ret = mmc_suspend_host(mmc); clk_disable_unprepare(host->clk_per); - clk_disable_unprepare(host->clk_ipg); + if (host->clk_ipg) + clk_disable_unprepare(host->clk_ipg); return ret; } @@ -1267,7 +1279,8 @@ static int mxcmci_resume(struct device *dev) int ret = 0; clk_prepare_enable(host->clk_per); - clk_prepare_enable(host->clk_ipg); + if (host->clk_ipg) + clk_prepare_enable(host->clk_ipg); if (mmc) ret = mmc_resume_host(mmc); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH RFC v2 0/5] MPC512x DMA slave s/g support, OF DMA lookup 2013-07-14 12:01 ` Gerhard Sittig ` (5 preceding siblings ...) (?) @ 2013-07-16 9:27 ` Alexander Popov -1 siblings, 0 replies; 45+ messages in thread From: Alexander Popov @ 2013-07-16 9:27 UTC (permalink / raw) To: Gerhard Sittig Cc: Lars-Peter Clausen, Arnd Bergmann, Vinod Koul, devicetree-discuss, Dan Williams, Anatolij Gustschin, linuxppc-dev Hello everyone! Hello Gerhard! Thanks for your work. 2013/7/14 Gerhard Sittig <gsi@denx.de>: > known issues: > - currently encoded constraints do work for SD card and LPB test suite > (all known DMA clients ATM), but will need more tuning or support for > automatic adjustment for transfers of arbitrary length NBYTES of data is read from / written to DMA client's port in one burst after DMA controller receives service request from that DMA client. Different DMA clients want different NBYTES: SCLPC wants 4 and SD card wants 64, other clients might want something different. So having default case with magic number 64 is totally wrong. What if we simply remove it from [PATCH RFC v2 2/5]: len = sg_dma_len(sg); if (mchan->tcd_nunits) tcd->nbytes = mchan->tcd_nunits * 4; - else - tcd->nbytes = 64; if (!IS_ALIGNED(len, tcd->nbytes)) return NULL; and make SD card driver use fields src_addr_width, dst_addr_width and src_maxburst / dst_maxburst of dma_slave_config which is a part of standard API (dmaengine.h): #define DEFAULT_WORDS_PER_TRANSFER 16 ... struct dma_slave_config dma_conf = {}; ... if (dma_conf.direction = DMA_MEM_TO_DEV) { dma_conf.dst_maxburst = DEFAULT_WORDS_PER_TRANSFER; } else { dma_conf.src_maxburst = DEFAULT_WORDS_PER_TRANSFER; } dma_conf.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; dma_conf.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; ... if (dma_dev->device_control(chan, DMA_SLAVE_CONFIG, (unsigned long)&dma_conf)) { goto err_dma_prep; } This code for SD card driver would be very similar to the code from SCLPC driver. Best regards, Alexander ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH RFC v2 0/5] MPC512x DMA slave s/g support, OF DMA lookup 2013-07-14 12:01 ` Gerhard Sittig ` (6 preceding siblings ...) (?) @ 2013-10-03 14:00 ` Alexander Popov 2013-10-06 10:01 ` Gerhard Sittig -1 siblings, 1 reply; 45+ messages in thread From: Alexander Popov @ 2013-10-03 14:00 UTC (permalink / raw) To: Gerhard Sittig Cc: Lars-Peter Clausen, Arnd Bergmann, Vinod Koul, devicetree-discuss, Alexander Popov, Dan Williams, Anatolij Gustschin, linuxppc-dev v2013/7/14 Gerhard Sittig <gsi@denx.de>: > this series > - introduces slave s/g support (that's support for DMA transfers which > involve peripherals in contrast to mem-to-mem transfers) > - adds device tree based lookup support for DMA channels > - combines floating patches and related feedback which already covered > several aspects of what the suggested LPB driver needs, to demonstrate > how integration might be done > - carries Q&D SD card support to enable another DMA client during test, > while this patch needs to get dropped upon pickup > > changes since v1: > - re-order mpc8308 related code paths for improved readability, no > change in behaviour, introduction of symbolic channel names here > already > - squash 'execute() start condition' and 'terminate all' into the > introduction of 'slave s/g prep' and 'device control' support; refuse > s/g lists with more than one item since slave support is operational > yet proper s/g support is missing (can get addressed later) > - always start transfers from software on MPC8308 as there are no > external request lines for peripheral flow control > - drop dt-bindings header file and symbolic channel names in OF nodes Changes since v2 (RFC v3 was badly formed, excuse me for that): Part 1/5: - use #define instead of enum since individual channels don't require special handling. Part 2/5: - add a flag "will_access_peripheral" to DMA transfer descriptor according recommendations of Gerhard Sittig. This flag is set in mpc_dma_prep_memcpy() and mpc_dma_prep_slave_sg() and is evaluated in mpc_dma_execute() to choose a type of start for the transfer. - prevent descriptors of transfers which involve peripherals from being chained together; each of such transfers needs hardware initiated start. - add locking while working with struct mpc_dma_chan according recommendations of Lars-Peter Clausen. - remove default nbytes value. Client kernel modules must set src_maxburst and dst_maxburst fields of struct dma_slave_config (dmaengine.h). Part 6/8: unchanged. Part 7/8: unchanged. Part 8/8: unchanged. These changes are tested on MPC5125 - with SCLPC driver (transfers between dev and mem work fine). - with dmatest module (all 64 DMA channels can perform mem-to-mem transfers which can be chained in one DMA transaction). > known issues: > - it's yet to get confirmed whether MPC8308 can use slave support or > whether the DMA controller's driver shall actively reject it, the > information that's available so far suggests that peripheral transfers > to IP bus attached I/O is useful and shall not get blocked right away > ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH RFC v2 0/5] MPC512x DMA slave s/g support, OF DMA lookup 2013-10-03 14:00 ` Alexander Popov @ 2013-10-06 10:01 ` Gerhard Sittig 0 siblings, 0 replies; 45+ messages in thread From: Gerhard Sittig @ 2013-10-06 10:01 UTC (permalink / raw) To: Alexander Popov Cc: Lars-Peter Clausen, Arnd Bergmann, Vinod Koul, devicetree-discuss, Dan Williams, Anatolij Gustschin, linuxppc-dev On Thu, Oct 03, 2013 at 18:00 +0400, Alexander Popov wrote: > > v2013/7/14 Gerhard Sittig <gsi@denx.de>: > > this series > > - introduces slave s/g support (that's support for DMA transfers which > > involve peripherals in contrast to mem-to-mem transfers) > > - adds device tree based lookup support for DMA channels > > - combines floating patches and related feedback which already covered > > several aspects of what the suggested LPB driver needs, to demonstrate > > how integration might be done > > - carries Q&D SD card support to enable another DMA client during test, > > while this patch needs to get dropped upon pickup > > > > changes since v1: > > - re-order mpc8308 related code paths for improved readability, no > > change in behaviour, introduction of symbolic channel names here > > already > > - squash 'execute() start condition' and 'terminate all' into the > > introduction of 'slave s/g prep' and 'device control' support; refuse > > s/g lists with more than one item since slave support is operational > > yet proper s/g support is missing (can get addressed later) > > - always start transfers from software on MPC8308 as there are no > > external request lines for peripheral flow control > > - drop dt-bindings header file and symbolic channel names in OF nodes > > Changes since v2 (RFC v3 was badly formed, excuse me for that): > Part 1/5: > - use #define instead of enum since individual channels don't require > special handling. > Part 2/5: > - add a flag "will_access_peripheral" to DMA transfer descriptor > according recommendations of Gerhard Sittig. > This flag is set in mpc_dma_prep_memcpy() and mpc_dma_prep_slave_sg() > and is evaluated in mpc_dma_execute() to choose a type of start for > the transfer. > - prevent descriptors of transfers which involve peripherals from > being chained together; > each of such transfers needs hardware initiated start. > - add locking while working with struct mpc_dma_chan > according recommendations of Lars-Peter Clausen. > - remove default nbytes value. Client kernel modules must set > src_maxburst and dst_maxburst fields of struct dma_slave_config (dmaengine.h). > Part 6/8: > unchanged. > Part 7/8: > unchanged. > Part 8/8: > unchanged. > > These changes are tested on MPC5125 > - with SCLPC driver (transfers between dev and mem work fine). > - with dmatest module (all 64 DMA channels can perform mem-to-mem transfers > which can be chained in one DMA transaction). > > > known issues: > > - it's yet to get confirmed whether MPC8308 can use slave support or > > whether the DMA controller's driver shall actively reject it, the > > information that's available so far suggests that peripheral transfers > > to IP bus attached I/O is useful and shall not get blocked right away I'm not certain whether keeping the "cover letter" in threaded form is appropriate. But speaking about non-existent parts (6-8/8), not saying what the current version is, missing stats certainly isn't right. Your submission style adds more work to doing review and providing feedback than what's necessary. You assume that others would keep the history for you, or would do the archeology and collect individual pieces from the past, to recover what you fail to send out. The messages show up in an unexpected order here (3, 4, 5, 0 first on one thread, 1, 2 then in another thread, with some 2800 messages between them) and carry conflicting subjects or version numbers. Formatting was broken in transport, patches won't apply. Please do consider checking what you send out, and how you can improve the reception's side of the process. Try to help those people you want to receive help from. Try to support both kinds of review for people coming new to the subject as well as those who have seen a former version. virtually yours Gerhard Sittig -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH RFC 4/8] dts: mpc512x: prepare for preprocessor support 2013-07-12 15:26 ` Gerhard Sittig ` (2 preceding siblings ...) (?) @ 2013-07-12 15:26 ` Gerhard Sittig -1 siblings, 0 replies; 45+ messages in thread From: Gerhard Sittig @ 2013-07-12 15:26 UTC (permalink / raw) To: linuxppc-dev, devicetree-discuss, Alexander Popov Cc: Lars-Peter Clausen, Arnd Bergmann, Vinod Koul, Gerhard Sittig, Dan Williams, Anatolij Gustschin prepare C preprocessor support when processing MPC512x DTS files - switch from DTS syntax to CPP syntax for include specs - create a symlink such that DTS processing can reference includes Signed-off-by: Gerhard Sittig <gsi@denx.de> --- arch/powerpc/boot/dts/ac14xx.dts | 2 +- arch/powerpc/boot/dts/include/dt-bindings | 1 + arch/powerpc/boot/dts/mpc5121ads.dts | 2 +- arch/powerpc/boot/dts/pdm360ng.dts | 2 +- 4 files changed, 4 insertions(+), 3 deletions(-) create mode 120000 arch/powerpc/boot/dts/include/dt-bindings diff --git a/arch/powerpc/boot/dts/ac14xx.dts b/arch/powerpc/boot/dts/ac14xx.dts index a27a460..a543c40 100644 --- a/arch/powerpc/boot/dts/ac14xx.dts +++ b/arch/powerpc/boot/dts/ac14xx.dts @@ -10,7 +10,7 @@ */ -/include/ "mpc5121.dtsi" +#include <mpc5121.dtsi> / { model = "ac14xx"; diff --git a/arch/powerpc/boot/dts/include/dt-bindings b/arch/powerpc/boot/dts/include/dt-bindings new file mode 120000 index 0000000..08c00e4 --- /dev/null +++ b/arch/powerpc/boot/dts/include/dt-bindings @@ -0,0 +1 @@ +../../../../../include/dt-bindings \ No newline at end of file diff --git a/arch/powerpc/boot/dts/mpc5121ads.dts b/arch/powerpc/boot/dts/mpc5121ads.dts index 7d3cb79..c228a0a 100644 --- a/arch/powerpc/boot/dts/mpc5121ads.dts +++ b/arch/powerpc/boot/dts/mpc5121ads.dts @@ -9,7 +9,7 @@ * option) any later version. */ -/include/ "mpc5121.dtsi" +#include <mpc5121.dtsi> / { model = "mpc5121ads"; diff --git a/arch/powerpc/boot/dts/pdm360ng.dts b/arch/powerpc/boot/dts/pdm360ng.dts index 7433740..871c16d 100644 --- a/arch/powerpc/boot/dts/pdm360ng.dts +++ b/arch/powerpc/boot/dts/pdm360ng.dts @@ -13,7 +13,7 @@ * option) any later version. */ -/include/ "mpc5121.dtsi" +#include <mpc5121.dtsi> / { model = "pdm360ng"; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH RFC 5/8] dma: mpc512x: use symbolic specifiers for DMA channels 2013-07-12 15:26 ` Gerhard Sittig ` (3 preceding siblings ...) (?) @ 2013-07-12 15:26 ` Gerhard Sittig [not found] ` <1373642781-32631-6-git-send-email-gsi-ynQEQJNshbs@public.gmane.org> -1 siblings, 1 reply; 45+ messages in thread From: Gerhard Sittig @ 2013-07-12 15:26 UTC (permalink / raw) To: linuxppc-dev, devicetree-discuss, Alexander Popov Cc: Lars-Peter Clausen, Arnd Bergmann, Vinod Koul, Gerhard Sittig, Dan Williams, Anatolij Gustschin for the DMA controller of the MPC512x SoC, DMA channels directly correspond to specific peripherals, since requester lines directly map to channels while no additional flexibility or mapping is involved introduce a dt-bindings header file for MPC512x DMA channels, and make the shared DT specs in the mpc5121.dtsi as well as the DMA engine driver use those names instead of numbers Signed-off-by: Gerhard Sittig <gsi@denx.de> --- arch/powerpc/boot/dts/mpc5121.dtsi | 6 +++++- drivers/dma/mpc512x_dma.c | 6 ++++-- include/dt-bindings/dma/mpc512x-dma.h | 21 +++++++++++++++++++++ 3 files changed, 30 insertions(+), 3 deletions(-) create mode 100644 include/dt-bindings/dma/mpc512x-dma.h diff --git a/arch/powerpc/boot/dts/mpc5121.dtsi b/arch/powerpc/boot/dts/mpc5121.dtsi index bd14c00..384e692 100644 --- a/arch/powerpc/boot/dts/mpc5121.dtsi +++ b/arch/powerpc/boot/dts/mpc5121.dtsi @@ -9,6 +9,8 @@ * option) any later version. */ +#include <dt-bindings/dma/mpc512x-dma.h> + /dts-v1/; / { @@ -152,7 +154,7 @@ compatible = "fsl,mpc5121-sdhc"; reg = <0x1500 0x100>; interrupts = <8 0x8>; - dmas = <&dma0 30>; + dmas = <&dma0 MPC512x_DMACHAN_SDHC>; dma-names = "rx-tx"; }; @@ -262,6 +264,8 @@ lpc@10000 { compatible = "fsl,mpc5121-lpc"; reg = <0x10000 0x200>; + dmas = <&dma0 MPC512x_DMACHAN_SCLPC>; + dma-names = "rx-tx"; }; pata@10200 { diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c index 0053ff8..8f6d545 100644 --- a/drivers/dma/mpc512x_dma.c +++ b/drivers/dma/mpc512x_dma.c @@ -29,6 +29,8 @@ * file called COPYING. */ +#include <dt-bindings/dma/mpc512x-dma.h> + #include <linux/module.h> #include <linux/dmaengine.h> #include <linux/dma-mapping.h> @@ -257,7 +259,7 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan) prev->tcd->dlast_sga = mdesc->tcd_paddr; prev->tcd->e_sg = 1; /* only start explicitly on MDDRC channel */ - if (cid == 32) + if (cid == MPC512x_DMACHAN_MDDRC) mdesc->tcd->start = 1; prev = mdesc; @@ -276,7 +278,7 @@ static void mpc_dma_execute(struct mpc_dma_chan *mchan) /* peripherals involved, use external request */ out_8(&mdma->regs->dmaserq, cid); break; - case 32: + case MPC512x_DMACHAN_MDDRC: /* memory transfer, software provided start signal */ out_8(&mdma->regs->dmassrt, cid); break; diff --git a/include/dt-bindings/dma/mpc512x-dma.h b/include/dt-bindings/dma/mpc512x-dma.h new file mode 100644 index 0000000..56b06d1 --- /dev/null +++ b/include/dt-bindings/dma/mpc512x-dma.h @@ -0,0 +1,21 @@ +/* + * This header file provides symbolic specifiers for DMA channels + * within the MPC512x SoC's DMA controller. Since requester lines + * directly map to channel numbers and no additional flexibility + * is involved, DMA channels can be considered directly associated + * with individual peripherals. + * + * This header file gets shared among DT bindings which provide + * hardware specs, and driver code which implements supporting logic. + */ + +#ifndef _DT_BINDINGS_DMA_MPC512x_DMA_H +#define _DT_BINDINGS_DMA_MPC512x_DMA_H + +#define MPC512x_DMACHAN_SCLPC 26 +#define MPC512x_DMACHAN_SDHC 30 +#define MPC512x_DMACHAN_MDDRC 32 + +#define MPC512x_DMACHAN_MAX 64 + +#endif -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 45+ messages in thread
[parent not found: <1373642781-32631-6-git-send-email-gsi-ynQEQJNshbs@public.gmane.org>]
* Re: [PATCH RFC 5/8] dma: mpc512x: use symbolic specifiers for DMA channels 2013-07-12 15:26 ` [PATCH RFC 5/8] dma: mpc512x: use symbolic specifiers for DMA channels Gerhard Sittig @ 2013-07-13 7:17 ` Arnd Bergmann 0 siblings, 0 replies; 45+ messages in thread From: Arnd Bergmann @ 2013-07-13 7:17 UTC (permalink / raw) To: Gerhard Sittig Cc: Lars-Peter Clausen, Vinod Koul, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Alexander Popov, Dan Williams, linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ On Friday 12 July 2013, Gerhard Sittig wrote: > +++ b/include/dt-bindings/dma/mpc512x-dma.h > @@ -0,0 +1,21 @@ > +/* > + * This header file provides symbolic specifiers for DMA channels > + * within the MPC512x SoC's DMA controller. Since requester lines > + * directly map to channel numbers and no additional flexibility > + * is involved, DMA channels can be considered directly associated > + * with individual peripherals. > + * > + * This header file gets shared among DT bindings which provide > + * hardware specs, and driver code which implements supporting logic. > + */ > + > +#ifndef _DT_BINDINGS_DMA_MPC512x_DMA_H > +#define _DT_BINDINGS_DMA_MPC512x_DMA_H > + > +#define MPC512x_DMACHAN_SCLPC 26 > +#define MPC512x_DMACHAN_SDHC 30 > +#define MPC512x_DMACHAN_MDDRC 32 > + > +#define MPC512x_DMACHAN_MAX 64 > + I think these should not be in the header and should not bve part of the binding either. They are specific to an SoC that happens to be using this DMA controller but would be completely different for any other SoC with the same DMA engine. These belong into the dma descriptors of the slave drivers and don't need symbolic names. Arnd ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH RFC 5/8] dma: mpc512x: use symbolic specifiers for DMA channels @ 2013-07-13 7:17 ` Arnd Bergmann 0 siblings, 0 replies; 45+ messages in thread From: Arnd Bergmann @ 2013-07-13 7:17 UTC (permalink / raw) To: Gerhard Sittig Cc: Lars-Peter Clausen, Vinod Koul, devicetree-discuss, Alexander Popov, Dan Williams, Anatolij Gustschin, linuxppc-dev On Friday 12 July 2013, Gerhard Sittig wrote: > +++ b/include/dt-bindings/dma/mpc512x-dma.h > @@ -0,0 +1,21 @@ > +/* > + * This header file provides symbolic specifiers for DMA channels > + * within the MPC512x SoC's DMA controller. Since requester lines > + * directly map to channel numbers and no additional flexibility > + * is involved, DMA channels can be considered directly associated > + * with individual peripherals. > + * > + * This header file gets shared among DT bindings which provide > + * hardware specs, and driver code which implements supporting logic. > + */ > + > +#ifndef _DT_BINDINGS_DMA_MPC512x_DMA_H > +#define _DT_BINDINGS_DMA_MPC512x_DMA_H > + > +#define MPC512x_DMACHAN_SCLPC 26 > +#define MPC512x_DMACHAN_SDHC 30 > +#define MPC512x_DMACHAN_MDDRC 32 > + > +#define MPC512x_DMACHAN_MAX 64 > + I think these should not be in the header and should not bve part of the binding either. They are specific to an SoC that happens to be using this DMA controller but would be completely different for any other SoC with the same DMA engine. These belong into the dma descriptors of the slave drivers and don't need symbolic names. Arnd ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH RFC 5/8] dma: mpc512x: use symbolic specifiers for DMA channels 2013-07-13 7:17 ` Arnd Bergmann (?) @ 2013-07-13 14:14 ` Gerhard Sittig 2013-07-14 8:50 ` Arnd Bergmann -1 siblings, 1 reply; 45+ messages in thread From: Gerhard Sittig @ 2013-07-13 14:14 UTC (permalink / raw) To: Arnd Bergmann Cc: Lars-Peter Clausen, Vinod Koul, devicetree-discuss, Alexander Popov, Dan Williams, Anatolij Gustschin, linuxppc-dev [ MPC8308 knowledge required, see below ] On Sat, Jul 13, 2013 at 09:17 +0200, Arnd Bergmann wrote: > > On Friday 12 July 2013, Gerhard Sittig wrote: > > +++ b/include/dt-bindings/dma/mpc512x-dma.h > > @@ -0,0 +1,21 @@ > > +/* > > + * This header file provides symbolic specifiers for DMA channels > > + * within the MPC512x SoC's DMA controller. Since requester lines > > + * directly map to channel numbers and no additional flexibility > > + * is involved, DMA channels can be considered directly associated > > + * with individual peripherals. > > + * > > + * This header file gets shared among DT bindings which provide > > + * hardware specs, and driver code which implements supporting logic. > > + */ > > + > > +#ifndef _DT_BINDINGS_DMA_MPC512x_DMA_H > > +#define _DT_BINDINGS_DMA_MPC512x_DMA_H > > + > > +#define MPC512x_DMACHAN_SCLPC 26 > > +#define MPC512x_DMACHAN_SDHC 30 > > +#define MPC512x_DMACHAN_MDDRC 32 > > + > > +#define MPC512x_DMACHAN_MAX 64 > > + > > I think these should not be in the header and should not bve part of the > binding either. They are specific to an SoC that happens to be using this > DMA controller but would be completely different for any other SoC with > the same DMA engine. These belong into the dma descriptors of the slave > drivers and don't need symbolic names. Thank you for the feedback. OK, so not adding the dt-bindings header leads to no change in the DTS nodes, which in turn collapses 5/8 into something local to the .c driver source (introduce an enum and replace a few magic numbers with names), and obsoletes 4/8 as a prerequisite. This will further reduce the patch set's size. Your feedback made me re-visit the driver source and notice that it is shared among the MPC512x as well as the MPC8308 hardware. The latter only has 16 channels, and appears to _not_ have its channels dedicated to peripherals. It's even uncertain to me whether it can cope with peripherals at all and how so. I scanned chapter 12 (DMA controller) in the MPC8308 reference manual (rev 0 as of 2010-04) several times and could not find any hint about peripherals, request lines, or anything else related to flow control. Searching in the whole RM won't give a hint either. Does this suggest that the MPC8308 DMA controller's channels are "free" in their assignment to transfer tasks? Or are they "memory transfers only"? Or do they happily accept any XLB address (internal and external RAM, IMMR and IP bus space) but don't apply flow control, i.e. expect either peripherals to already hold the RX data, or peripherals to keep up with being fed random amounts of TX data? I tend to read the doc as the latter. Can somebody with MPC8308 knowledge confirm my suspicion that the MPC8308 DMA channels aren't associated with peripherals, and thus always need the software initiated start condition (_if_ they are used for data transfer to/from peripherals)? Can somebody with access to an MPC8308 based board test later versions of the series, to verify we don't break DMA operation on that platform? Regardless of whether MPC8308 can't handle peripheral access, or doesn't differ from memory transfers there, the patch series needs an update. Part 1/8 in its current form is either wrong or incomplete, works for MPC512x but not for all hardware that this driver is responsible for. virtually yours Gerhard Sittig -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH RFC 5/8] dma: mpc512x: use symbolic specifiers for DMA channels 2013-07-13 14:14 ` Gerhard Sittig @ 2013-07-14 8:50 ` Arnd Bergmann 2013-07-14 9:53 ` Lars-Peter Clausen 2013-07-14 11:02 ` Gerhard Sittig 0 siblings, 2 replies; 45+ messages in thread From: Arnd Bergmann @ 2013-07-14 8:50 UTC (permalink / raw) To: Gerhard Sittig Cc: Lars-Peter Clausen, Vinod Koul, devicetree-discuss, Alexander Popov, Dan Williams, Anatolij Gustschin, linuxppc-dev On Saturday 13 July 2013, Gerhard Sittig wrote: > [ MPC8308 knowledge required, see below ] > > On Sat, Jul 13, 2013 at 09:17 +0200, Arnd Bergmann wrote: > > > > On Friday 12 July 2013, Gerhard Sittig wrote: > > > +++ b/include/dt-bindings/dma/mpc512x-dma.h > > > @@ -0,0 +1,21 @@ > > > +/* > > > + * This header file provides symbolic specifiers for DMA channels > > > + * within the MPC512x SoC's DMA controller. Since requester lines > > > + * directly map to channel numbers and no additional flexibility > > > + * is involved, DMA channels can be considered directly associated > > > + * with individual peripherals. > > > + * > > > + * This header file gets shared among DT bindings which provide > > > + * hardware specs, and driver code which implements supporting logic. > > > + */ > > > + > > > +#ifndef _DT_BINDINGS_DMA_MPC512x_DMA_H > > > +#define _DT_BINDINGS_DMA_MPC512x_DMA_H > > > + > > > +#define MPC512x_DMACHAN_SCLPC 26 > > > +#define MPC512x_DMACHAN_SDHC 30 > > > +#define MPC512x_DMACHAN_MDDRC 32 > > > + > > > +#define MPC512x_DMACHAN_MAX 64 > > > + > > > > I think these should not be in the header and should not bve part of the > > binding either. They are specific to an SoC that happens to be using this > > DMA controller but would be completely different for any other SoC with > > the same DMA engine. These belong into the dma descriptors of the slave > > drivers and don't need symbolic names. > > Thank you for the feedback. > > OK, so not adding the dt-bindings header leads to no change in > the DTS nodes, which in turn collapses 5/8 into something local > to the .c driver source (introduce an enum and replace a few > magic numbers with names), and obsoletes 4/8 as a prerequisite. > This will further reduce the patch set's size. Actually I think you will need extra changes: The dma-engine driver should not require knowledge of any channel-specific settings. I did not notice you had them until you mentioned the above, but from what I can tell, you need a few flags in the dma-specifier to replace code like /* only start explicitly on MDDRC channel */ - if (cid == 32) + if (cid == MPC512x_DMACHAN_MDDRC) mdesc->tcd->start = 1; with mdesc->tcd->start = dmaspec->explicit_start; or something along these lines, where dmaspec is a data structure derived from the fields in the DT dma specifier of the child node. > I scanned chapter 12 (DMA controller) in the MPC8308 reference > manual (rev 0 as of 2010-04) several times and could not find any > hint about peripherals, request lines, or anything else related > to flow control. Searching in the whole RM won't give a hint > either. Does this suggest that the MPC8308 DMA controller's > channels are "free" in their assignment to transfer tasks? Or > are they "memory transfers only"? Or do they happily accept any > XLB address (internal and external RAM, IMMR and IP bus space) > but don't apply flow control, i.e. expect either peripherals to > already hold the RX data, or peripherals to keep up with being > fed random amounts of TX data? I tend to read the doc as the > latter. It sounds to me that they are memory-to-memory only, which means you probably want to allow #dma-cells=<0> as a special case to describe an instance that has no slave API support. Arnd ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH RFC 5/8] dma: mpc512x: use symbolic specifiers for DMA channels 2013-07-14 8:50 ` Arnd Bergmann @ 2013-07-14 9:53 ` Lars-Peter Clausen 2013-07-14 11:02 ` Gerhard Sittig 1 sibling, 0 replies; 45+ messages in thread From: Lars-Peter Clausen @ 2013-07-14 9:53 UTC (permalink / raw) To: Arnd Bergmann Cc: Vinod Koul, Gerhard Sittig, Alexander Popov, Dan Williams, Anatolij Gustschin, linuxppc-dev, devicetree-discuss On 07/14/2013 10:50 AM, Arnd Bergmann wrote: > On Saturday 13 July 2013, Gerhard Sittig wrote: >> [ MPC8308 knowledge required, see below ] >> >> On Sat, Jul 13, 2013 at 09:17 +0200, Arnd Bergmann wrote: >>> >>> On Friday 12 July 2013, Gerhard Sittig wrote: >>>> +++ b/include/dt-bindings/dma/mpc512x-dma.h >>>> @@ -0,0 +1,21 @@ >>>> +/* >>>> + * This header file provides symbolic specifiers for DMA channels >>>> + * within the MPC512x SoC's DMA controller. Since requester lines >>>> + * directly map to channel numbers and no additional flexibility >>>> + * is involved, DMA channels can be considered directly associated >>>> + * with individual peripherals. >>>> + * >>>> + * This header file gets shared among DT bindings which provide >>>> + * hardware specs, and driver code which implements supporting logic. >>>> + */ >>>> + >>>> +#ifndef _DT_BINDINGS_DMA_MPC512x_DMA_H >>>> +#define _DT_BINDINGS_DMA_MPC512x_DMA_H >>>> + >>>> +#define MPC512x_DMACHAN_SCLPC 26 >>>> +#define MPC512x_DMACHAN_SDHC 30 >>>> +#define MPC512x_DMACHAN_MDDRC 32 >>>> + >>>> +#define MPC512x_DMACHAN_MAX 64 >>>> + >>> >>> I think these should not be in the header and should not bve part of the >>> binding either. They are specific to an SoC that happens to be using this >>> DMA controller but would be completely different for any other SoC with >>> the same DMA engine. These belong into the dma descriptors of the slave >>> drivers and don't need symbolic names. >> >> Thank you for the feedback. >> >> OK, so not adding the dt-bindings header leads to no change in >> the DTS nodes, which in turn collapses 5/8 into something local >> to the .c driver source (introduce an enum and replace a few >> magic numbers with names), and obsoletes 4/8 as a prerequisite. >> This will further reduce the patch set's size. > > Actually I think you will need extra changes: The dma-engine driver > should not require knowledge of any channel-specific settings. > I did not notice you had them until you mentioned the above, but > from what I can tell, you need a few flags in the dma-specifier > to replace code like > > /* only start explicitly on MDDRC channel */ > - if (cid == 32) > + if (cid == MPC512x_DMACHAN_MDDRC) > mdesc->tcd->start = 1; > > with > > mdesc->tcd->start = dmaspec->explicit_start; > > or something along these lines, where dmaspec is a data structure > derived from the fields in the DT dma specifier of the child > node. > I think the MDDRC channel is used for mem-to-mem transfers. So there is no peripheral that is going to request it by handle. If the channel that is used for mem-to-mem transfers differs between different instances of the controller (e.g. on different SoCs) it should probably be a property of the dma controllers DT node. - Lars ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH RFC 5/8] dma: mpc512x: use symbolic specifiers for DMA channels 2013-07-14 8:50 ` Arnd Bergmann 2013-07-14 9:53 ` Lars-Peter Clausen @ 2013-07-14 11:02 ` Gerhard Sittig 1 sibling, 0 replies; 45+ messages in thread From: Gerhard Sittig @ 2013-07-14 11:02 UTC (permalink / raw) To: Arnd Bergmann Cc: Lars-Peter Clausen, Vinod Koul, devicetree-discuss, Alexander Popov, Dan Williams, Anatolij Gustschin, linuxppc-dev On Sun, Jul 14, 2013 at 10:50 +0200, Arnd Bergmann wrote: > > On Saturday 13 July 2013, Gerhard Sittig wrote: > > > > [ ... ] > > > > Thank you for the feedback. > > > > OK, so not adding the dt-bindings header leads to no change in > > the DTS nodes, which in turn collapses 5/8 into something local > > to the .c driver source (introduce an enum and replace a few > > magic numbers with names), and obsoletes 4/8 as a prerequisite. > > This will further reduce the patch set's size. > > Actually I think you will need extra changes: The dma-engine driver > should not require knowledge of any channel-specific settings. > I did not notice you had them until you mentioned the above, but > from what I can tell, you need a few flags in the dma-specifier > to replace code like > > /* only start explicitly on MDDRC channel */ > - if (cid == 32) > + if (cid == MPC512x_DMACHAN_MDDRC) > mdesc->tcd->start = 1; > > with > > mdesc->tcd->start = dmaspec->explicit_start; > > or something along these lines, where dmaspec is a data structure > derived from the fields in the DT dma specifier of the child > node. The above change applies to the mpc512x_dma.c file, this is the very driver source which implements the DMA engine API, and deals with the specific details of the SoC. So I consider this the most appropriate location to handle requirements of specific channels in the DMA controller's driver, while the generic DMA engine subsystem just finds the provider's API. The driver takes care of _one_ DMA controller which has several channels while these channels in turn might have _different_ characteristics. Most channels of the MPC512x SoC's DMA controller have request lines for peripherals to apply flow control, while the 'MDDRC' channel is dedicated to mem-to-mem transfers and requires a software initiated start in the absence of an external request line. The channels of the MPC8308 SoC's DMA controller appear to not have any request lines, which makes them perfectly fit for mem-to-mem transfers and always requires software initiated start. Yet this does not absolutely prevent their use for peripheral access, _provided_ that the peripheral's FIFO is deep enough or the data volume is appropriately low (or wire speed is rather high, whatever obsoletes flow control). I'm in the process of preparing v2 of the series, which keeps compatibility with the MPC8308 and approaches things slightly differently than v1 although in the same spirit. Regarding the device tree binding, I was under the impression that backwards compatibility is a must. Reading channel counts from DT nodes instead of encoding them in the driver would have been nice upon introduction of OF support, but would break compatibility these days. > > I scanned chapter 12 (DMA controller) in the MPC8308 reference > > manual (rev 0 as of 2010-04) several times and could not find any > > hint about peripherals, request lines, or anything else related > > to flow control. Searching in the whole RM won't give a hint > > either. Does this suggest that the MPC8308 DMA controller's > > channels are "free" in their assignment to transfer tasks? Or > > are they "memory transfers only"? Or do they happily accept any > > XLB address (internal and external RAM, IMMR and IP bus space) > > but don't apply flow control, i.e. expect either peripherals to > > already hold the RX data, or peripherals to keep up with being > > fed random amounts of TX data? I tend to read the doc as the > > latter. > > It sounds to me that they are memory-to-memory only, which means > you probably want to allow #dma-cells=<0> as a special case to > describe an instance that has no slave API support. This would impact the current OF registration, which unconditionally assumes one-cell specs. Before changing this to one-cell for MPC512x and zero-zell for MPC8308, I'd love to learn whether slave support on MPC8308 is inapplicable or whether it's doable yet constraints apply (ATM I assume the latter, would not want to enforce non-availability just to re-introduce it later). virtually yours Gerhard Sittig -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH RFC 7/8] dma: mpc512x: register for device tree channel lookup 2013-07-12 15:26 ` Gerhard Sittig ` (4 preceding siblings ...) (?) @ 2013-07-12 15:26 ` Gerhard Sittig -1 siblings, 0 replies; 45+ messages in thread From: Gerhard Sittig @ 2013-07-12 15:26 UTC (permalink / raw) To: linuxppc-dev, devicetree-discuss, Alexander Popov Cc: Lars-Peter Clausen, Arnd Bergmann, Vinod Koul, Gerhard Sittig, Dan Williams, Anatolij Gustschin register the controller for device tree based lookup of DMA channels (non-fatal for backwards compatibility with older device trees), provide the '#dma-cells' property in the shared mpc5121.dtsi file, and introduce a bindings document for the MPC512x DMA controller Signed-off-by: Gerhard Sittig <gsi@denx.de> --- .../devicetree/bindings/dma/mpc512x-dma.txt | 55 ++++++++++++++++++++ arch/powerpc/boot/dts/mpc5121.dtsi | 1 + drivers/dma/mpc512x_dma.c | 21 ++++++-- 3 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 Documentation/devicetree/bindings/dma/mpc512x-dma.txt diff --git a/Documentation/devicetree/bindings/dma/mpc512x-dma.txt b/Documentation/devicetree/bindings/dma/mpc512x-dma.txt new file mode 100644 index 0000000..a4867d5 --- /dev/null +++ b/Documentation/devicetree/bindings/dma/mpc512x-dma.txt @@ -0,0 +1,55 @@ +* Freescale MPC512x DMA Controller + +The DMA controller in the Freescale MPC512x SoC can move blocks of +memory contents between memory and peripherals or memory to memory. + +Refer to the "Generic DMA Controller and DMA request bindings" description +in the dma.txt file for a more detailled discussion of the binding. The +MPC512x DMA engine binding follows the common scheme, but doesn't provide +support for the optional channels and requests counters (those values are +derived from the detected hardware features) and has a fixed client +specifier length of 1 integer cell (the value is the DMA channel, since +the DMA controller uses a fixed assignment of request lines per channel). + + +DMA controller node properties: + +Required properties: +- compatible: should be "fsl,mpc5121-dma" +- reg: address and size of the DMA controller's register set +- interrupts: interrupt spec for the DMA controller + +Optional properties: +- #dma-cells: must be <1>, describes the number of integer cells + needed to specify the 'dmas' property in client nodes, + strongly recommended since common client helper code + uses this property + +Example: + + dma0: dma@14000 { + compatible = "fsl,mpc5121-dma"; + reg = <0x14000 0x1800>; + interrupts = <65 0x8>; + #dma-cells = <1>; + }; + + +Client node properties: + +Required properties: +- dmas: list of DMA specifiers, consisting each of a handle + for the DMA controller and integer cells to specify + the channel used within the DMA controller +- dma-names: list of identifier strings for the DMA specifiers, + client device driver code uses these strings to + have DMA channels looked up at the controller + +Example: + + sdhc@1500 { + compatible = "fsl,mpc5121-sdhc"; + /* ... */ + dmas = <&dma0 30>; + dma-names = "rx-tx"; + }; diff --git a/arch/powerpc/boot/dts/mpc5121.dtsi b/arch/powerpc/boot/dts/mpc5121.dtsi index 384e692..dae99b7 100644 --- a/arch/powerpc/boot/dts/mpc5121.dtsi +++ b/arch/powerpc/boot/dts/mpc5121.dtsi @@ -394,6 +394,7 @@ compatible = "fsl,mpc5121-dma"; reg = <0x14000 0x1800>; interrupts = <65 0x8>; + #dma-cells = <1>; }; }; diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c index 8f6d545..3d79e3a 100644 --- a/drivers/dma/mpc512x_dma.c +++ b/drivers/dma/mpc512x_dma.c @@ -38,6 +38,7 @@ #include <linux/io.h> #include <linux/slab.h> #include <linux/of_device.h> +#include <linux/of_dma.h> #include <linux/of_platform.h> #include <linux/random.h> @@ -939,11 +940,23 @@ static int mpc_dma_probe(struct platform_device *op) /* Register DMA engine */ dev_set_drvdata(dev, mdma); retval = dma_async_device_register(dma); - if (retval) { - devm_free_irq(dev, mdma->irq, mdma); - irq_dispose_mapping(mdma->irq); + if (retval) + goto out_irq; + + /* register with OF helpers for DMA lookups (nonfatal) */ + if (dev->of_node) { + retval = of_dma_controller_register(dev->of_node, + of_dma_xlate_by_chan_id, + mdma); + if (retval) + dev_warn(dev, "could not register for OF lookup\n"); } + return 0; + +out_irq: + devm_free_irq(dev, mdma->irq, mdma); + irq_dispose_mapping(mdma->irq); return retval; } @@ -952,6 +965,8 @@ static int mpc_dma_remove(struct platform_device *op) struct device *dev = &op->dev; struct mpc_dma *mdma = dev_get_drvdata(dev); + if (dev->of_node) + of_dma_controller_free(dev->of_node); dma_async_device_unregister(&mdma->dma); devm_free_irq(dev, mdma->irq, mdma); irq_dispose_mapping(mdma->irq); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH RFC 8/8] HACK mmc: mxcmmc: enable clocks for the MPC512x 2013-07-12 15:26 ` Gerhard Sittig ` (5 preceding siblings ...) (?) @ 2013-07-12 15:26 ` Gerhard Sittig -1 siblings, 0 replies; 45+ messages in thread From: Gerhard Sittig @ 2013-07-12 15:26 UTC (permalink / raw) To: linuxppc-dev, devicetree-discuss, Alexander Popov Cc: Lars-Peter Clausen, Arnd Bergmann, Vinod Koul, Gerhard Sittig, Dan Williams, Anatolij Gustschin Q&D HACK to enable SD card support without correct COMMON_CLK support, best viewed with 'git diff -w -b', NOT acceptable for mainline (NAKed) Signed-off-by: Gerhard Sittig <gsi@denx.de> --- drivers/mmc/host/mxcmmc.c | 41 +++++++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c index d503635..b9d21cc 100644 --- a/drivers/mmc/host/mxcmmc.c +++ b/drivers/mmc/host/mxcmmc.c @@ -1121,20 +1121,29 @@ static int mxcmci_probe(struct platform_device *pdev) host->res = r; host->irq = irq; - host->clk_ipg = devm_clk_get(&pdev->dev, "ipg"); - if (IS_ERR(host->clk_ipg)) { - ret = PTR_ERR(host->clk_ipg); - goto out_iounmap; - } + if (!is_mpc512x_mmc(host)) { + host->clk_ipg = devm_clk_get(&pdev->dev, "ipg"); + if (IS_ERR(host->clk_ipg)) { + ret = PTR_ERR(host->clk_ipg); + goto out_iounmap; + } - host->clk_per = devm_clk_get(&pdev->dev, "per"); - if (IS_ERR(host->clk_per)) { - ret = PTR_ERR(host->clk_per); - goto out_iounmap; + host->clk_per = devm_clk_get(&pdev->dev, "per"); + if (IS_ERR(host->clk_per)) { + ret = PTR_ERR(host->clk_per); + goto out_iounmap; + } + } else { + host->clk_per = devm_clk_get(&pdev->dev, "sdhc_clk"); + if (IS_ERR(host->clk_per)) { + ret = PTR_ERR(host->clk_per); + goto out_iounmap; + } } clk_prepare_enable(host->clk_per); - clk_prepare_enable(host->clk_ipg); + if (host->clk_ipg) + clk_prepare_enable(host->clk_ipg); mxcmci_softreset(host); @@ -1204,7 +1213,8 @@ out_free_dma: dma_release_channel(host->dma); out_clk_put: clk_disable_unprepare(host->clk_per); - clk_disable_unprepare(host->clk_ipg); + if (host->clk_ipg) + clk_disable_unprepare(host->clk_ipg); out_iounmap: iounmap(host->base); out_free: @@ -1236,7 +1246,8 @@ static int mxcmci_remove(struct platform_device *pdev) dma_release_channel(host->dma); clk_disable_unprepare(host->clk_per); - clk_disable_unprepare(host->clk_ipg); + if (host->clk_ipg) + clk_disable_unprepare(host->clk_ipg); release_mem_region(host->res->start, resource_size(host->res)); @@ -1255,7 +1266,8 @@ static int mxcmci_suspend(struct device *dev) if (mmc) ret = mmc_suspend_host(mmc); clk_disable_unprepare(host->clk_per); - clk_disable_unprepare(host->clk_ipg); + if (host->clk_ipg) + clk_disable_unprepare(host->clk_ipg); return ret; } @@ -1267,7 +1279,8 @@ static int mxcmci_resume(struct device *dev) int ret = 0; clk_prepare_enable(host->clk_per); - clk_prepare_enable(host->clk_ipg); + if (host->clk_ipg) + clk_prepare_enable(host->clk_ipg); if (mmc) ret = mmc_resume_host(mmc); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 45+ messages in thread
end of thread, other threads:[~2013-10-06 10:01 UTC | newest] Thread overview: 45+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-07-12 15:26 [PATCH RFC 0/8] MPC512x DMA slave s/g support, OF DMA lookup Gerhard Sittig 2013-07-12 15:26 ` Gerhard Sittig 2013-07-12 15:26 ` [PATCH RFC 1/8] powerpc: mpc512x_dma: add support for data transfers between memory and i/o memory Gerhard Sittig 2013-07-14 10:05 ` Lars-Peter Clausen [not found] ` <51E277E5.8050305-Qo5EllUWu/uELgA04lAiVw@public.gmane.org> 2013-07-14 11:07 ` Gerhard Sittig 2013-07-14 11:07 ` Gerhard Sittig [not found] ` <1373642781-32631-1-git-send-email-gsi-ynQEQJNshbs@public.gmane.org> 2013-07-12 15:26 ` [PATCH RFC 2/8] dma: mpc512x: fix start condition in execute() Gerhard Sittig 2013-07-12 15:26 ` Gerhard Sittig 2013-07-12 15:26 ` [PATCH RFC 3/8] dma: mpc512x: support 'terminate all' control request Gerhard Sittig 2013-07-12 15:26 ` Gerhard Sittig 2013-07-12 15:26 ` [PATCH RFC 6/8] dma: of: Add common xlate function for matching by channel id Gerhard Sittig 2013-07-12 15:26 ` Gerhard Sittig 2013-07-12 16:45 ` [PATCH RFC 0/8] MPC512x DMA slave s/g support, OF DMA lookup Lars-Peter Clausen 2013-07-12 16:45 ` Lars-Peter Clausen 2013-07-14 12:01 ` [PATCH RFC v2 0/5] " Gerhard Sittig 2013-07-14 12:01 ` Gerhard Sittig 2013-07-14 12:01 ` [PATCH RFC v2 1/5] dma: mpc512x: re-order mpc8308 specific instructions Gerhard Sittig 2013-08-12 13:38 ` Alexander Popov 2013-08-12 13:38 ` Alexander Popov 2013-07-14 12:01 ` [PATCH RFC v2 2/5] dma: mpc512x: add support for peripheral transfers Gerhard Sittig 2013-07-16 10:37 ` Lars-Peter Clausen [not found] ` <51E5226D.9050100-Qo5EllUWu/uELgA04lAiVw@public.gmane.org> 2013-07-17 10:42 ` Gerhard Sittig 2013-07-17 10:42 ` Gerhard Sittig 2013-07-31 7:46 ` Alexander Popov 2013-08-12 13:37 ` Alexander Popov 2013-08-12 13:37 ` Alexander Popov 2013-07-14 12:01 ` [PATCH RFC v2 3/5] dma: of: Add common xlate function for matching by channel id Gerhard Sittig 2013-10-03 14:05 ` Alexander Popov 2013-07-14 12:02 ` [PATCH RFC v2 4/5] dma: mpc512x: register for device tree channel lookup Gerhard Sittig 2013-10-03 14:06 ` Alexander Popov 2013-07-14 12:02 ` [PATCH RFC v2 5/5] HACK mmc: mxcmmc: enable clocks for the MPC512x Gerhard Sittig 2013-10-03 14:06 ` Alexander Popov 2013-07-16 9:27 ` [PATCH RFC v2 0/5] MPC512x DMA slave s/g support, OF DMA lookup Alexander Popov 2013-10-03 14:00 ` Alexander Popov 2013-10-06 10:01 ` Gerhard Sittig 2013-07-12 15:26 ` [PATCH RFC 4/8] dts: mpc512x: prepare for preprocessor support Gerhard Sittig 2013-07-12 15:26 ` [PATCH RFC 5/8] dma: mpc512x: use symbolic specifiers for DMA channels Gerhard Sittig [not found] ` <1373642781-32631-6-git-send-email-gsi-ynQEQJNshbs@public.gmane.org> 2013-07-13 7:17 ` Arnd Bergmann 2013-07-13 7:17 ` Arnd Bergmann 2013-07-13 14:14 ` Gerhard Sittig 2013-07-14 8:50 ` Arnd Bergmann 2013-07-14 9:53 ` Lars-Peter Clausen 2013-07-14 11:02 ` Gerhard Sittig 2013-07-12 15:26 ` [PATCH RFC 7/8] dma: mpc512x: register for device tree channel lookup Gerhard Sittig 2013-07-12 15:26 ` [PATCH RFC 8/8] HACK mmc: mxcmmc: enable clocks for the MPC512x Gerhard Sittig
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.