[lkml]   [2012]   [Mar]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH/RFC] dmaengine: add a slave parameter to __dma_request_channel()
    On Wed, Mar 07, 2012 at 04:44:12PM +0100, Guennadi Liakhovetski wrote:
    > So, the question remains: which way should we go? If we don't come up with
    > a generic solution, I'd be inclined to just do something as silly as
    > arch/arm/mach-shmobile/board-*.c
    > static const struct device *group1_dma_dev[] = {
    > &dma0.device,
    > &dma1.device,
    > };
    > static const struct device *group2_dma_dev[] = {
    > &dma2.device,
    > &dma3.device,
    > };
    > static struct my_platform_data = {
    > .dma_dev_list = group1_dma_dev,
    > .dma_dev_list_num = ARRAY_SIZE(group1_dma_dev),
    > };
    > drivers/.../sh_*.c
    > static bool filter(struct dma_chan *chan, void *arg)
    > {
    > struct device *dev = chan->dev->device.parent;
    > struct my_platform_data *pdata = arg;
    > for (i = 0; i < pdata->dma_dev_list_num; i++)
    > if (pdata->dma_dev_list[i] == dev)
    > return true;
    > return false;
    > }
    > even though I find it silly to have to do this on every platform and in
    > every driver.

    Why are you thinking that the filter function implementation has to be
    provided by the peripheral driver? That's just wrong and broken.

    Think about it - how does the peripheral driver know what kind of dma
    channel its filter function has been passed - to give an example, if
    you built into your kernel support for the PL08x DMA engine, and lets
    say you had PL08x DMA engine hardware, how would your filter function
    decide whether it was one of your per-device channels or whether it
    was a PL08x DMA engine channel?

    And in the case of a PL08x DMA engine channel (which for slaves are
    all virtual channels depending on the request signal desired), how
    would your peripheral driver know which channel would be the right

    This is why DMA engine drivers such as PL08x and SA11x0 export their
    own filter functions. The filter function will only match against a
    channel owned by that exact driver - essentially this:

    bool xxx_filter_id(struct dma_chan *chan, void *chan_id)
    /* Reject channels for devices not bound to this driver */
    if (chan->device->dev->driver != &xxx_driver)
    return false;

    ... xxx device specific channel matching ...

    return false;

    This ensures that when the filter function needs to dereference the
    driver private parts of struct dma_chan, it can be sure that the channel
    really is being driven by this driver, and the underlying structure
    behind struct dma_chan is well known at that point.

    Anything which isn't driven by this driver is (as far as the filter
    function is concerned) an unknown.

    Once you know that the channel is owned by the right driver you can then
    take whatever action is necessary to check - with type safety - whether
    the channel is correct or not. You get type safety through two facts:

    1. As described above, the filter function will reject any channel not
    driven by this driver.
    2. You need to pass the drivers filter function and data together into
    the same API. Because both are together you have locality of

    And... moreover, matching struct devices just would not fit for any DMA
    engine I've worked with - that being PL08x, SA11x0 and OMAP. Neither
    does, as I've said, using the dma_slave_config struct.

    Let me put it another way:

    1. using struct device would be wrong for these DMA engines because a
    single DMA engine _device_ has multiple physical DMA channels, and
    not only do these physical channels share registers and interrupts,
    but they also share DMA requests.

    2. the dma_slave_config struct can't be used to select a DMA engine or
    a DMA channel because it does not contain information about whether
    the DMA engine can access the peripheral, whether there's a link for
    the device request signals to the DMA engine channel.

    I will give you that channel matching in DMA engine stuff is far from
    perfect, but what I'm seeing are attempts to optimize the channel
    allocation for a different set of DMA devices at the expense of
    penalising those already there.

    We could pass a struct device corresponding with the desired DMA engine
    and DMA request number in, but that still won't fit the PL08x driver,
    where there's a platform specific hardware mux in front of a subset of
    the PL08x's request signals.

    So, what this is all telling me is that the _right_ interface is the one
    we currently have, that being the filter function and an opaque set of
    data to be interpreted by the supplied filter function. If you need to
    match by a list of struct device, then the interface allows it. If you
    need to match by string against a channel driven by a specific DMA engine
    driver, the interface allows it. If you need to do something more
    complicated, like one of N drivers, the interface allows it.

    Hell, even with the PL08x and SA11x0 filter functions, lets say for
    arguments sake that I had hardware with both these in, I could select
    a channel like this:

    bool pl08x_or_sa11x0_filter(struct dma_chan *chan, void *chan_id)
    if (pl08x_filter_id(chan, chan_id))
    return true;

    return sa11x0_filter_id(chan, chan_id);

    where both treat 'chan_id' as a string.

     \ /
      Last update: 2012-03-07 17:31    [W:0.037 / U:2.784 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site