On Tuesday 18 June 2013, Guennadi Liakhovetski wrote:
> >
> > Hmm, you've clearly shown that this can work, but it feels like a really
> > odd way to
> > do this. I don't think we should do it this way, because it tries to be
> > really
> > generic but then cannot some of the interesting cases, e.g.
> >
> > 1. you have multiplexed dma engines, but they need different #dma-cells.
> > 1. you have multiplexed dma engines, but they need different dma specifiers.
> > 2. The mux is between devices that are not on the same parent bus.
>
> Yes, I know about these restrictions. I'm not sure whether we really ever
> will get DMA multiplexers with different #dma-cells or DMA specifiers, but
> in any case we can make this less generic - either keep it as a generic
> "uniform multiplexer" or making it shdma specific - I'm fine either way.
Ok, let's make it shdma specific then.
> > I think this should either not try to be fully generic and just limited to
> > the case you care about, i.e. shdma, or it should be more abstract and
> > multiplex between the individual channels. In either case, I guess
> > it would not need a change like this to the of_dma_request_slave_channel()
> > function, and the node pointer used by the slave would be the same node
> > that defines the address space for the channel descriptions with #dma-cells.
> >
> > I think the easiest way would be the first of those two, so it would
> > essentially look like
> >
> > dmac: dma-mux {
> > compatible = "renesas,shdma-mux"; /* not simple-bus! */
> > #dma-cells = <1>;
> > #address-cells = <1>;
> > #size-cells = <1>;
> >
> > dma@10000000 {
> > compatible = "renesas,shdma";
> > reg = <0x10000000 0x1000>;
> > interrupts = <10>;
> > };
> > dma@20000000 {
> > compatible = "renesas,shdma";
> > reg = <0x10000000 0x1000>;
> > interrupts = <10>;
> > };
> > }
> >
> > You then register a device driver for the master device, which
> > will call of_dma_controller_register() for itself and create
> > its child devices.
>
> Hmm, it is an interesting idea to only register one struct of_dma instance
> for all compatible shdma instances instead of one per shdma controller,
> and then call of_platform_populate() to instantiate all shdma instances. A
> couple of drawbacks:
>
> 1. we'll always have to put a mux DT node in .dts, even if there's just
> one DMAC instance on the system.
>
> 2. as AUXDATA for the new wrapper device we'll have to use an AUXDATA
> array for all child nodes, to be passed to of_platform_populate().
My suggestion above is just one of the possible ways to do it, and I'm
less concerned about it if you make it specific to shdma. Other
options would be:
1 using phandles from the mux to the individual dma engine instances,
but have them as independent nodes.
2 same as 1, but using phandles from the individual instances to the mux
3 Keep using the simple-bus.
4 Have just one combined device node for all shdma instances, with multiple
'reg' and 'interrupts' properties, and handle the muxing in the shdma
driver. You could create platform_device_create_resndata from the
top-level driver to reuse most of the existing code.
In any of these cases you should be able to support both muxed and non-muxed
operation in the shdma driver if you want to. You'd just have two separate
ofdma registrations.
> 3. it seems confusing to me - having one ofdma instance for multiple
> dmaengine devices.
I don't see a better way around this one, but I also don't see it as
problematic.
Wiht a mux, you always end up having just one node that the slaves
point to, but multiple dma_device structures in the kernel. struct ofdma
really refers to the first one.
> The advantage is, of course, that we don't need to extend existing OF and
> dmaengine APIs.
>
> So, I think, it can be done this way, but do you really think it'd be an
> improvement over my original proposal?
My main interest is to keep the shdma specifics out of the generic dmaengine
binding document, where it just complicates the common case.
Arnd
_______________________________________________
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
