On Wed, Apr 25, 2018 at 10:08:17PM +0200, Angelo Dureghello wrote: > This patch adds dma support for NXP mcf5441x (ColdFire) family. > > ColdFire mcf5441x implements an edma hw module similar to the
Is it similar to to edma ? > one implemented in Vybrid VFxxx controllers, but with a slightly > different register set, more dma channels (64 instead of 32), > a different interrupt mechanism and some other minor differences. > > For the above reasons, modfying fsl-edma.c was too complex and > likely too ugly. From here, the decision to create a different > driver, but starting from fsl-edma. can the common stuff be made into a lib and shared between then two rather than having a same driver or different drivers? > > The driver has been tested with mcf5441x (stmark2 board) and > dspi driver, it worked fine and seems reliable at least as a > first initial version. > > Signed-off-by: Angelo Dureghello <[email protected]> > --- > arch/m68k/configs/stmark2_defconfig | 2 + this should be a separate patch please > multiplexing capability for DMA request sources(slot). > This module can be found on Freescale Vybrid and LS-1 SoCs. > > +config MCF_EDMA Alphabetical sort pls > +// SPDX-License-Identifier: GPL-2.0 Copyright info should be here in c99 style comments > + > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt why do you need this, why not use dev_xxx > +#define EDMA_CHANNELS 64 > +#define EDMA_MASK_CH(x) ((x) & 0x3F) > +#define EDMA_MASK_ITER(x) ((x) & 0x7FFF) > +#define EDMA_TCD_MEM_ALIGN 32 > + > +#define EDMA_TCD_ATTR_DSZ_8b (0x0000) > +#define EDMA_TCD_ATTR_DSZ_16b (0x0001) > +#define EDMA_TCD_ATTR_DSZ_32b (0x0002) > +#define EDMA_TCD_ATTR_DSZ_16B (0x0004) BIT and GENMASK for these.. > +static unsigned int mcf_edma_get_tcd_attr(enum dma_slave_buswidth addr_width) > +{ > + switch (addr_width) { > + case 1: > + return EDMA_TCD_ATTR_SSZ_8b | EDMA_TCD_ATTR_DSZ_8b; > + case 2: > + return EDMA_TCD_ATTR_SSZ_16b | EDMA_TCD_ATTR_DSZ_16b; > + case 4: > + default: why default not treated as error? > +static int mcf_edma_slave_config(struct dma_chan *chan, > + struct dma_slave_config *cfg) > +{ > + struct mcf_edma_chan *mcf_chan = to_mcf_edma_chan(chan); > + > + mcf_chan->esc.dir = cfg->direction; > + if (cfg->direction == DMA_DEV_TO_MEM) { > + mcf_chan->esc.dev_addr = cfg->src_addr; > + mcf_chan->esc.addr_width = cfg->src_addr_width; > + mcf_chan->esc.burst = cfg->src_maxburst; > + mcf_chan->esc.attr = mcf_edma_get_tcd_attr(cfg->src_addr_width); > + } else if (cfg->direction == DMA_MEM_TO_DEV) { > + mcf_chan->esc.dev_addr = cfg->dst_addr; > + mcf_chan->esc.addr_width = cfg->dst_addr_width; > + mcf_chan->esc.burst = cfg->dst_maxburst; > + mcf_chan->esc.attr = mcf_edma_get_tcd_attr(cfg->dst_addr_width); please save both src/dstn details here, typically we dont at this point about the txn direction... direction comes with prep_xxx call > +static void mcf_edma_free_desc(struct virt_dma_desc *vdesc) > +{ > + struct mcf_edma_desc *mcf_desc; > + int i; > + struct edma_regs *regs; > + > + mcf_desc = to_mcf_edma_desc(vdesc); > + regs = mcf_desc->echan->edma->membase; > + > + //trace_tcd(®s->tcd[mcf_desc->echan->slave_id]); > + //trace_regs(regs); ?? > +static int mcf_edma_irq_init(struct platform_device *pdev, > + struct mcf_edma_engine *mcf_edma) > +{ > + int ret = 0, i; > + struct resource *res; > + > + res = platform_get_resource_byname(pdev, > + IORESOURCE_IRQ, "edma-tx-00-15"); > + if (!res) > + return -1; > + > + for (ret = 0, i = res->start; i <= res->end; ++i) { > + ret |= devm_request_irq(&pdev->dev, i, > + mcf_edma_tx_handler, 0, "eDMA", mcf_edma); you are explicitly freeing irq below, so why use devm_ ? -- ~Vinod -- To unsubscribe from this list: send the line "unsubscribe linux-m68k" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
