On 29 January 2013 03:28, Arnd Bergmann <a...@arndb.de> wrote: > The original device tree binding for this driver, from Viresh Kumar > unfortunately conflicted with the generic DMA binding, and did not allow > to completely seperate slave device configuration from the controller. > > This is an attempt to replace it with an implementation of the generic > binding, but it is currently completely untested, because I do not have > any hardware with this particular controller. > > The patch applies on top of linux-next, which contains both the base > support for the generic DMA binding, as well as the earlier attempt from > Viresh. Both of these are currently not merged upstream however.
This was really my work and i am feeling bad that i couldn't allocate any time for it. Thanks for starting it. > diff --git a/Documentation/devicetree/bindings/dma/snps-dma.txt > b/Documentation/devicetree/bindings/dma/snps-dma.txt > index 5bb3dfb..212d387 100644 > --- a/Documentation/devicetree/bindings/dma/snps-dma.txt > +++ b/Documentation/devicetree/bindings/dma/snps-dma.txt > @@ -3,59 +3,62 @@ > Required properties: > - compatible: "snps,dma-spear1340" > - reg: Address range of the DMAC registers > -- interrupt-parent: Should be the phandle for the interrupt controller > - that services interrupts for this device > - interrupt: Should contain the DMAC interrupt number > -- nr_channels: Number of channels supported by hardware > -- is_private: The device channels should be marked as private and not for by > the > - general purpose DMA channel allocator. False if not passed. > +- dma-channels: Number of channels supported by hardware > +- dma-requests: Number of DMA request lines supported > +- dma-masters: Number of AHB masters supported by the controller > +- #dma-cells: must be <3> Shouldn't this be 4? Would be better to mention what fields are these, right here. I have seen them below though. > +DMA clients connected to the Designware DMA controller must use the format > +described in the dma.txt file, using a five-cell specifier for each channel. > +The five cells in order are: > + > +1. A phandle pointing to the DMA controller > +2. The value for the cfg_hi register. > +3. The value for the cfg_lo register. > +4. Source master for transfers on allocated channel. > +5. Destination master for transfers on allocated channel. > diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c > index 8cfaaf8..88504c2 100644 > --- a/drivers/dma/dw_dmac.c > +++ b/drivers/dma/dw_dmac.c > @@ -18,6 +18,7 @@ > #include <linux/interrupt.h> > #include <linux/io.h> > #include <linux/of.h> > +#include <linux/of_dma.h> > #include <linux/mm.h> > #include <linux/module.h> > #include <linux/platform_device.h> > @@ -1179,49 +1180,69 @@ static void dwc_free_chan_resources(struct dma_chan > *chan) > dev_vdbg(chan2dev(chan), "%s: done\n", __func__); > } > > -bool dw_dma_generic_filter(struct dma_chan *chan, void *param) > +/* forward declaration used in filter */ > +static struct platform_driver dw_driver; extern? This is not a declaration but definition. > + > +struct dw_dma_filter_args { > + struct dw_dma *dw; > + u32 cfg_lo; > + u32 cfg_hi; > + unsigned src; Currently named as sms > + unsigned dst; dms > +}; > + > +static bool dw_dma_generic_filter(struct dma_chan *chan, void *param) > { > struct dw_dma *dw = to_dw_dma(chan->device); > - static struct dw_dma *last_dw; > - static char *last_bus_id; > - int i = -1; > + struct dw_dma_filter_args *fargs = param; > + struct dw_dma_slave *sd; > > - /* > - * dmaengine framework calls this routine for all channels of all dma > - * controller, until true is returned. If 'param' bus_id is not > - * registered with a dma controller (dw), then there is no need of > - * running below function for all channels of dw. > - * > - * This block of code does this by saving the parameters of last > - * failure. If dw and param are same, i.e. trying on same dw with > - * different channel, return false. > - */ > - if ((last_dw == dw) && (last_bus_id == param)) > + /* both the driver and the device must match */ > + if (chan->device->dev->driver != &dw_driver.driver) > + return false; Can this ever happen? Isn't it the case that this routine would be called only for dw_dmac? > + if (dw != fargs->dw) > return false; > - /* > - * Return true: > - * - If dw_dma's platform data is not filled with slave info, then all > - * dma controllers are fine for transfer. > - * - Or if param is NULL > - */ > - if (!dw->sd || !param) > - return true; > > - while (++i < dw->sd_count) { > - if (!strcmp(dw->sd[i].bus_id, param)) { > - chan->private = &dw->sd[i]; > - last_dw = NULL; > - last_bus_id = NULL; > + /* FIXME: memory leak! could we put this into dw_dma_chan? */ > + sd = devm_kzalloc(dw->dma.dev, sizeof (*sd), GFP_KERNEL); Yes. > + if (!sd) > + return false; > > - return true; > - } > - } > + sd->dma_dev = dw->dma.dev; > + sd->cfg_hi = fargs->cfg_hi; > + sd->cfg_lo = fargs->cfg_lo; > + sd->src_master = fargs->src; > + sd->dst_master = fargs->dst; > + > + chan->private = sd; > > - last_dw = dw; > - last_bus_id = param; > - return false; > + return true; > +} > + > +static struct dma_chan *dw_dma_xlate(struct of_phandle_args *dma_spec, > + struct of_dma *ofdma) > +{ > + struct dw_dma *dw = ofdma->of_dma_data; > + struct dw_dma_filter_args fargs = { > + .dw = dw, > + }; > + dma_cap_mask_t cap; > + > + if (dma_spec->args_count != 4) args_count contains count of all params leaving the phandle? > + return NULL; > + > + /* FIXME: This binding is rather clumsy. Can't we use the > + request line numbers here instead? */ yes. > + fargs.cfg_lo = be32_to_cpup(dma_spec->args+0); > + fargs.cfg_hi = be32_to_cpup(dma_spec->args+1); > + fargs.src = be32_to_cpup(dma_spec->args+2); > + fargs.dst = be32_to_cpup(dma_spec->args+3); > + > + dma_cap_zero(cap); > + dma_cap_set(DMA_SLAVE, cap); > + /* FIXME: there should be a simpler way to do this */ > + return dma_request_channel(cap, dw_dma_generic_filter, > &dma_spec->args[0]); don't you need to send &fargs as the last argument? _______________________________________________ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss