Re: [PATCH V3 1/3] dmaengine: dw-axi-dmac: support DMAX_NUM_CHANNELS > 8
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2021-10-26 12:10:01
Hi Pandith, On Fri, Oct 1, 2021 at 4:13 PM [off-list ref] wrote:
From: Pandith N <redacted> Added support for DMA controller with more than 8 channels. DMAC register map changes based on number of channels. Enabling DMAC channel: DMAC_CHENREG has to be used when number of channels <= 8 DMAC_CHENREG2 has to be used when number of channels > 8 Configuring DMA channel: CHx_CFG has to be used when number of channels <= 8 CHx_CFG2 has to be used when number of channels > 8 Suspending and resuming channel: DMAC_CHENREG has to be used when number of channels <= 8 DMAC_CHSUSPREG has to be used for suspending a channel > 8 Signed-off-by: Pandith N <redacted> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Thanks for your patch, which is now commit 824351668a413af7
("dmaengine: dw-axi-dmac: support DMAX_NUM_CHANNELS > 8") in
dmaengine/next.
--- Changes V1-->V2: Initialize register values in channel resume and pause Removed unwanted braces in flow control setting. Changes from v2->v3 check if channel is enabled, before suspending.
I couldn't find these versions in the archive, so I don't know if my comments below were made before...
quoted hunk ↗ jump to hunk
--- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
quoted hunk ↗ jump to hunk
@@ -1120,10 +1150,17 @@ static int dma_chan_pause(struct dma_chan *dchan) spin_lock_irqsave(&chan->vc.lock, flags); - val = axi_dma_ioread32(chan->chip, DMAC_CHEN); - val |= BIT(chan->id) << DMAC_CHAN_SUSP_SHIFT | - BIT(chan->id) << DMAC_CHAN_SUSP_WE_SHIFT; - axi_dma_iowrite32(chan->chip, DMAC_CHEN, val); + if (chan->chip->dw->hdata->reg_map_8_channels) { + val = axi_dma_ioread32(chan->chip, DMAC_CHEN); + val |= BIT(chan->id) << DMAC_CHAN_SUSP_SHIFT | + BIT(chan->id) << DMAC_CHAN_SUSP_WE_SHIFT; + axi_dma_iowrite32(chan->chip, DMAC_CHEN, val); + } else { + val = 0;
So unlike for the DMAC_CHEN register, you don't have to retain the old values for the DMAC_CHSUSPREG register?
+ val |= BIT(chan->id) << DMAC_CHAN_SUSP2_SHIFT | + BIT(chan->id) << DMAC_CHAN_SUSP2_WE_SHIFT;
Why not "val = BIT(...) | BIT(...)"?
+ axi_dma_iowrite32(chan->chip, DMAC_CHSUSPREG, val);
+ }
do {
if (axi_chan_irq_read(chan) & DWAXIDMAC_IRQ_SUSPENDED)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds