dmaengine: mcf-edma: add ColdFire mcf5441x eDMA support

[Vinod Koul <vinod.koul@intel.com>]

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 <angelo@sysam.it>
> ---
>  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(&regs->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_ ?