On 07/31/2018 07:56 AM, Arnd Bergmann wrote:
> On Fri, Jul 27, 2018 at 6:20 PM, Rob Landley <r...@landley.net> wrote:
>> On 07/24/2018 03:21 PM, Christoph Hellwig wrote:
>>> On Tue, Jul 24, 2018 at 02:01:42PM +0200, Christoph Hellwig wrote:
>>>> Hi all,
>>>>
>>>> can you review these patches to switch sh to use the generic
>>>> dma-noncoherent code? All the requirements are in mainline already
>>>> and we've switched various architectures over to it already.
>>>
>>> Ok, there is one more issue with this version. Wait for a new one
>>> tomorrow.
>>
>> Speaking of DMA:
>>
>> I'm trying to wire up DMAEngine to an sh7760 board that uses platform data
>> (and
>> fix the smc91x.c driver to use DMAEngine without #ifdef arm), so I've been
>> reading through all that stuff, but the docs seem kinda... thin?
>>
>> Is there something I should have read other than
>> Documentation/driver-model/platform.txt,
>> Documentation/dmaegine/{provider,client}.txt, then trying to picking through
>> the
>> source code and the sh7760 hardware pdf? (And watching the youtube video of
>> Laurent Pinchart's 2014 ELC talk on DMA, Maxime Ripard's 2015 ELC overview of
>> DMAEngine, the Xilinx video on DMAEngine...)
>>
>> At first I thought the SH_DMAE could initialize itself, but the probe
>> function
>> needs platform data, and although arch/sh/kernel/cpu/sh4a/setup-sh7722.c
>> looks
>> _kind_ of like a model I can crib from:
>
>> B) That platform data is supplying sh_dmae_slave_config preallocating slave
>> channels to devices? (Does it have to? The docs gave me the impression the
>> driver would dynamically request them and devices could even share. Wasn't
>> that
>> sort of the point of DMAEngine? Can my new board data _not_ do that? What's
>> the
>> minimum amount of micromanaging I have to do?)
>
> The thing here is that arch/sh is way behind on the API use, and it
> has prevented us from cleaning up drivers as well. A slave driver
> should have to just call dma_request_chan() with a constant
> string to identify its channel rather than going two different ways
> depending on whether it's used with DT or platform data.
I got the DMA controller hooked up to DMAEngine and the dmatest module is happy
with the result on all 8 channels. (Finding
arch/sh/kernel/cpu/sh4a/setup-sh7722.c helped a lot, finding it earlier would
have helped more. :)
The config symbols are:
CONFIG_SH_DMA=y
CONFIG_DMADEVICES=y
CONFIG_SH_DMAE_BASE=y
CONFIG_SH_DMAE=y
CONFIG_DMATEST=y #optional
The platform data is:
#include <cpu/dma-register.h>
#include <cpu/dma.h>
#include <linux/sh_dma.h>
/* DMA */
static struct resource sh7760_dma_resources[] = {
{
.start = SH_DMAC_BASE0,
.end = SH_DMAC_BASE0 + 9*16 - 1,
.flags = IORESOURCE_MEM,
}, {
.start = DMTE0_IRQ,
.end = DMTE0_IRQ,
.flags = IORESOURCE_IRQ,
}
};
static struct sh_dmae_channel dma_chan[] = {
{
.offset = 0,
.dmars = 0,
.dmars_bit = 0,
}, {
.offset = 0x10,
.dmars = 0,
.dmars_bit = 8,
}, {
.offset = 0x20,
.dmars = 4,
.dmars_bit = 0,
}, {
.offset = 0x30,
.dmars = 4,
.dmars_bit = 8,
}, {
.offset = 0x50,
.dmars = 8,
.dmars_bit = 0,
}, {
.offset = 0x60,
.dmars = 8,
.dmars_bit = 8,
}, {
.offset = 0x70,
.dmars = 12,
.dmars_bit = 0,
}, {
.offset = 0x80,
.dmars = 12,
.dmars_bit = 8,
}
};
static const unsigned int ts_shift[] = TS_SHIFT;
static struct sh_dmae_pdata sh7760_dma_pdata = {
.channel = dma_chan,
.channel_num = ARRAY_SIZE(dma_chan),
.ts_low_shift = CHCR_TS_LOW_SHIFT,
.ts_low_mask = CHCR_TS_LOW_MASK,
.ts_high_shift = CHCR_TS_HIGH_SHIFT,
.ts_high_mask = CHCR_TS_HIGH_MASK,
.ts_shift = ts_shift,
.ts_shift_num = ARRAY_SIZE(ts_shift),
.dmaor_init = DMAOR_INIT,
.dmaor_is_32bit = 1,
};
struct platform_device sh7760_dma_device = {
.name = "sh-dma-engine",
.id = -1,
.num_resources = ARRAY_SIZE(sh7760_dma_resources),
.resource = sh7760_dma_resources,
.dev = {.platform_data = &sh7760_dma_pdata},
};
And then add sh7760_dma_device to your struct platform_device array.
> If you hack on it, please convert the dmaengine platform data to use
> a dma_slave_map array to pass the data into the dmaengine driver,
The dmatest module didn't need it? I don't see why the ethernet driver would?
(Isn't the point of an allocator to allocate from a request?)
> mapping the settings from a (pdev-name, channel-id) tuple to a pointer
> that describes the channel configuration rather than having the
> mapping from an numerical slave_id to a struct sh_dmae_slave_config
> in the setup files. It should be a fairly mechanical conversion.
I think all 8 channels are generic. Drivers should be able to grab them and
release them at will, why does it need a table?
(I say this not having made the smc91x.c driver use this yet, its "conversion"
to device tree left it full of PXA #ifdefs and constants, and I've tried the
last half-dozen kernel releases and qemu releases and have yet to find an arm
mainstone board under qemu that _doesn't_ time out trying to use DMA with this
card. But that's another post...)
> The other part I noticed is arch/sh/drivers/dma/*, which appears to
> be entirely unused, and should probably removed.
I had to switch that off to get this to work, yes. I believe it predates
dmaengine and was obsoleted by it.
> Arnd
>
Rob
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
