Re: [PATCH 02/15] ARM: pxa: add dma slave map

[Robert Jarzmik]

Arnd Bergmann <arnd@xxxxxxxx> writes:

>> +       { "smc911x.0", "rx", PDMA_FILTER_PARAM(LOWEST, -1) },
>> +       { "smc911x.0", "tx", PDMA_FILTER_PARAM(LOWEST, -1) },
>> +       { "smc91x.0", "data", PDMA_FILTER_PARAM(LOWEST, -1) },
>
> This one is interesting, as you are dealing with an off-chip device,
> and the channel number is '-'1. How does this even work? Does it
> mean

This relies on pxa_dma, in which the "-1" for a requestor line means "no
requestor" or said in another way "always requesting". As a consequence, as soon
as the DMA descriptors are queued, the transfer begins, and it is supposed
implicitely that the FIFO output availability is at least as quick as the system
bus and the DMA size is perfectly fit for the FIFO available bytes.

This is what has been the underlying of DMA transfers of smc91x(x) on the PXA
platforms, where the smc91x(s) are directly wired on the system bus (the same
bus having DRAM, SRAM, IO-mapped devices).

>
>> +       /* PXA25x specific map */
>> +       { "pxa25x-ssp.0", "rx", PDMA_FILTER_PARAM(LOWEST, 13) },
>> +       { "pxa25x-ssp.0", "tx", PDMA_FILTER_PARAM(LOWEST, 14) },
>> +       { "pxa25x-nssp.1", "rx", PDMA_FILTER_PARAM(LOWEST, 15) },
>> +       { "pxa25x-nssp.1", "tx", PDMA_FILTER_PARAM(LOWEST, 16) },
>> +       { "pxa25x-nssp.2", "rx", PDMA_FILTER_PARAM(LOWEST, 23) },
>> +       { "pxa25x-nssp.2", "tx", PDMA_FILTER_PARAM(LOWEST, 24) },
>> +       { "pxa-pcm-audio", "nssp2_rx", PDMA_FILTER_PARAM(LOWEST, 15) },
>> +       { "pxa-pcm-audio", "nssp2_tx", PDMA_FILTER_PARAM(LOWEST, 16) },
>> +       { "pxa-pcm-audio", "nssp3_rx", PDMA_FILTER_PARAM(LOWEST, 23) },
>> +       { "pxa-pcm-audio", "nssp3_tx", PDMA_FILTER_PARAM(LOWEST, 24) },
>> +
>> +       /* PXA27x specific map */
>> +       { "pxa-pcm-audio", "ssp3_rx", PDMA_FILTER_PARAM(LOWEST, 66) },
>> +       { "pxa-pcm-audio", "ssp3_tx", PDMA_FILTER_PARAM(LOWEST, 67) },
>> +       { "pxa27x-camera.0", "CI_Y", PDMA_FILTER_PARAM(HIGHEST, 68) },
>> +       { "pxa27x-camera.0", "CI_U", PDMA_FILTER_PARAM(HIGHEST, 69) },
>> +       { "pxa27x-camera.0", "CI_V", PDMA_FILTER_PARAM(HIGHEST, 70) },
>> +
>> +       /* PXA3xx specific map */
>> +       { "pxa-pcm-audio", "ssp4_rx", PDMA_FILTER_PARAM(LOWEST, 2) },
>> +       { "pxa-pcm-audio", "ssp4_tx", PDMA_FILTER_PARAM(LOWEST, 3) },
>> +       { "pxa2xx-mci.1", "rx", PDMA_FILTER_PARAM(LOWEST, 93) },
>> +       { "pxa2xx-mci.1", "tx", PDMA_FILTER_PARAM(LOWEST, 94) },
>> +       { "pxa3xx-nand", "data", PDMA_FILTER_PARAM(LOWEST, 97) },
>> +       { "pxa2xx-mci.2", "rx", PDMA_FILTER_PARAM(LOWEST, 100) },
>> +       { "pxa2xx-mci.2", "tx", PDMA_FILTER_PARAM(LOWEST, 101) },
>> +};
>
> Since more than half the entries in here are chip specific, maybe it would be
> better to split that table into three and have a copy for each one in
> arch/arm/mach-pxa/pxa{25x.27x.3xx}.c?
Mmmh, today the split is :
 - 16 common entries
 - 10 pxa25x specific entries
 - 5 pxa27x specific entries
 - 7 pxa3xx specific entries
 => total of 38 lines

After the split we'll have :
 - 26 pxa25x specific entries
 - 21 pxa27x specific entries
 - 23 pxa3xx specific entries
 => total of 70 lines

That doubles the number of lines, not counting the declarations, and amending of
pxa2xx_set_dmac_info().

If you think it's worth it, what is the driving benefit behind ?

> Does that mean it's actually a memory-to-memory transfer with a device being
> on the external SRAM interface?
I'm taking this is the follow up to the "-1" question :0

Cheers.

-- 
Robert