Hi Pandith,
On Fri, Oct 1, 2021 at 4:13 PM <pandith.n@xxxxxxxxx> wrote:
> From: Pandith N <pandith.n@xxxxxxxxx>
>
> 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 <pandith.n@xxxxxxxxx>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
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...
> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> @@ -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@xxxxxxxxxxxxxx
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
