Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1578188 > unrolled thread

Re: [PATCH 2/2] dmaengine: Add DW AXI DMAC driver

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/2] dmaengine: Add DW AXI DMAC driver Vinod Koul <vinod.koul@intel.com> - 2017-02-10 07:10 +0100
    Re: [PATCH 2/2] dmaengine: Add DW AXI DMAC driver Alexey Brodkin <Alexey.Brodkin@synopsys.com> - 2017-02-10 09:30 +0100

#1578188 — Re: [PATCH 2/2] dmaengine: Add DW AXI DMAC driver

On Wed, Jan 25, 2017 at 06:34:17PM +0300, Eugeniy Paltsev wrote:
> This patch adds support for the DW AXI DMAC controller.
> 
> DW AXI DMAC is a part of upcoming development board from Synopsys.

How different is AXI from DW Synopsys?

Is the spec publicly available?

> +config AXI_DW_DMAC
> +	tristate "Synopsys DesignWare AXI DMA support"
> +	depends on OF && !64BIT

why not 64 bit, can you also add compile test

> +#define AXI_DMA_BUSWIDTHS		  \
> +	(DMA_SLAVE_BUSWIDTH_UNDEFINED	| \

DMA_SLAVE_BUSWIDTH_UNDEFINED??

> +static irqreturn_t dw_axi_dma_intretupt(int irq, void *dev_id)
> +{
> +	struct axi_dma_chip *chip = dev_id;
> +
> +	/* Disable DMAC inerrupts. We'll enable them in the tasklet */
> +	axi_dma_irq_disable(chip);
> +
> +	tasklet_schedule(&chip->dw->tasklet);

This is very inefficient, we would want to submit next txn here and not in
tasklet

> +static void axi_chan_block_xfer_complete(struct axi_dma_chan *chan)
> +{
> +	struct virt_dma_desc *vd;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&chan->vc.lock, flags);
> +	if (unlikely(axi_chan_is_hw_enable(chan))) {
> +		dev_err(chan2dev(chan), "BUG: %s catched DWAXIDMAC_IRQ_DMA_TRF, but channel not idle!\n",
> +			axi_chan_name(chan));
> +		axi_chan_disable(chan);
> +	}
> +
> +	/* The completed descriptor currently is in the head of vc list */
> +	vd = vchan_next_desc(&chan->vc);
> +	/* Remove the completed descriptor from issued list before completing */
> +	list_del(&vd->node);
> +	vchan_cookie_complete(vd);

this should be called from irq handler


> +static int dw_remove(struct platform_device *pdev)
> +{
> +	struct axi_dma_chip *chip = platform_get_drvdata(pdev);
> +	struct dw_axi_dma *dw = chip->dw;
> +	struct axi_dma_chan *chan, *_chan;
> +	u32 i;
> +
> +	axi_dma_irq_disable(chip);
> +	for (i = 0; i < dw->hdata->nr_channels; i++) {
> +		axi_chan_disable(&chip->dw->chan[i]);
> +		axi_chan_irq_disable(&chip->dw->chan[i], DWAXIDMAC_IRQ_ALL);
> +	}
> +	axi_dma_disable(chip);
> +
> +	tasklet_kill(&dw->tasklet);
> +
> +	list_for_each_entry_safe(chan, _chan, &dw->dma.channels,
> +			vc.chan.device_node) {
> +		list_del(&chan->vc.chan.device_node);
> +		tasklet_kill(&chan->vc.task);
> +	}

very nice :)

But please freeup irq as well

> +module_platform_driver(dw_driver);
> +
> +static int __init dw_init(void)
> +{
> +	return platform_driver_register(&dw_driver);
> +}
> +subsys_initcall(dw_init);

why subsys_initcall??

> +/* Common registers offset */
> +#define DMAC_ID			0x000 // R DMAC ID
> +#define DMAC_COMPVER		0x008 // R DMAC Component Version
> +#define DMAC_CFG		0x010 // R/W DMAC Configuration
> +#define DMAC_CHEN		0x018 // R/W DMAC Channel Enable
> +#define DMAC_CHEN_L		0x018 // R/W DMAC Channel Enable 00-31
> +#define DMAC_CHEN_H		0x01C // R/W DMAC Channel Enable 32-63
> +#define DMAC_INTSTATUS		0x030 // R DMAC Interrupt Status
> +#define DMAC_COMMON_INTCLEAR	0x038 // W DMAC Interrupt Clear
> +#define DMAC_COMMON_INTSTATUS_ENA 0x040 // R DMAC Interrupt Status Enable
> +#define DMAC_COMMON_INTSIGNAL_ENA 0x048 // R/W DMAC Interrupt Signal Enable
> +#define DMAC_COMMON_INTSTATUS	0x050 // R DMAC Interrupt Status
> +#define DMAC_RESET		0x058 // R DMAC Reset Register1

DMAC is a generic term, AX_DMAC and no C99 style comment, checkpatch would
have cribbed

Use BITS and GENMASK here

> +
> +/* DMA channel registers offset */
> +#define CH_SAR			0x000 // R/W Chan Source Address
> +#define CH_DAR			0x008 // R/W Chan Destination Address
> +#define CH_BLOCK_TS		0x010 // R/W Chan Block Transfer Size
> +#define CH_CTL			0x018 // R/W Chan Control
> +#define CH_CTL_L		0x018 // R/W Chan Control 00-31
> +#define CH_CTL_H		0x01C // R/W Chan Control 32-63
> +#define CH_CFG			0x020 // R/W Chan Configuration
> +#define CH_CFG_L		0x020 // R/W Chan Configuration 00-31
> +#define CH_CFG_H		0x024 // R/W Chan Configuration 32-63
> +#define CH_LLP			0x028 // R/W Chan Linked List Pointer
> +#define CH_STATUS		0x030 // R Chan Status
> +#define CH_SWHSSRC		0x038 // R/W Chan SW Handshake Source
> +#define CH_SWHSDST		0x040 // R/W Chan SW Handshake Destination
> +#define CH_BLK_TFR_RESUMEREQ	0x048 // W Chan Block Transfer Resume Req
> +#define CH_AXI_ID		0x050 // R/W Chan AXI ID
> +#define CH_AXI_QOS		0x058 // R/W Chan AXI QOS
> +#define CH_SSTAT		0x060 // R Chan Source Status
> +#define CH_DSTAT		0x068 // R Chan Destination Status
> +#define CH_SSTATAR		0x070 // R/W Chan Source Status Fetch Addr
> +#define CH_DSTATAR		0x078 // R/W Chan Destination Status Fetch Addr
> +#define CH_INTSTATUS_ENA	0x080 // R/W Chan Interrupt Status Enable
> +#define CH_INTSTATUS		0x088 // R/W Chan Interrupt Status
> +#define CH_INTSIGNAL_ENA	0x090 // R/W Chan Interrupt Signal Enable
> +#define CH_INTCLEAR		0x098 // W Chan Interrupt Clear

Same here

-- 
~Vinod

[toc] | [next] | [standalone]

#1578272

Hi Vinod,

On Fri, 2017-02-10 at 11:36 +0530, Vinod Koul wrote:
> On Wed, Jan 25, 2017 at 06:34:17PM +0300, Eugeniy Paltsev wrote:
> > 
> > This patch adds support for the DW AXI DMAC controller.
> > 
> > DW AXI DMAC is a part of upcoming development board from Synopsys.
> 
> How different is AXI from DW Synopsys?

These are 2 completely unrelated products from Synopsys, see:
 a) https://www.synopsys.com/dw/ipdir.php?ds=amba_ahb_dma
 b) https://www.synopsys.com/dw/ipdir.php?ds=amba_axi_dma

> Is the spec publicly available?

I'm afraid not. Synopsys customers may get DW AXI DMAC databook here:
https://www.synopsys.com/dw/doc.php/iip/DW_axi_dmac/latest/doc/DW_axi_dmac_databook.pdf

Just a side note "DW" prefix stands for DesignWare and applicable
to most if not all IP libs from Synopsys so here we're talking about
AHB DMAC vs AXI DMAC. Still my understanding is different bus type is
not the one and only difference but just the one which might simplify
selection of one or another IP block.

-Alexey

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web