Message-ID: <58bf2f7c-faf1-d1e9-3c87-d7964d1ba3d4@st.com>
Date:   Fri, 4 Aug 2017 14:50:01 +0200
From:   Pierre Yves MORDRET <pierre-yves.mordret@...com>
To:     Peter Ujfalusi <peter.ujfalusi@...com>,
        Vinod Koul <vinod.koul@...el.com>
CC:     Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Alexandre TORGUE <alexandre.torgue@...com>,
        Russell King <linux@...linux.org.uk>,
        Dan Williams <dan.j.williams@...el.com>,
        "M'boumba Cedric Madianga" <cedric.madianga@...il.com>,
        Fabrice GASNIER <fabrice.gasnier@...com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Fabien DESSENNE <fabien.dessenne@...com>,
        Amelie DELAUNAY <amelie.delaunay@...com>,
        "dmaengine@...r.kernel.org" <dmaengine@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 2/5] dmaengine: Add STM32 DMAMUX driver



On 08/03/2017 11:48 AM, Peter Ujfalusi wrote:
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 
> On 2017-08-03 12:00, Pierre Yves MORDRET wrote:
>>> What I actually mean is that you should not need to modify the DMA 
>>> driver at all.
>>> According to stm32-dma.txt:
>>> #dma-cells = <4>;
>>> 1. channelID
>>> 2. request line number
>>> 3. - 4. some parameters
>>>
>>> I believe if you don't have the event router (directly using the DMA 
>>> node) you always need to provide these, right?
>>
>> Correct
>>
>>> If I'm not mistaken, when you use the event router you want to omit the 
>>> ChannelID and get a random channel via dma_get_any_slave_channel in the 
>>> DMA driver and feed back the channelID to the event router driver?
>>
>> Again correct.
>>
>>> I believe the reason for this is that you want to keep mixed binding 
>>> use, to have direct DMA bindings and bindings via event router.
>>>
>>
>> Well no. peripheral has to use DMAMUX and mixing up is to be avoided. This is
>> more for backward compatibility with SoC which doesn't have a DMAMUX.
>>
>>> Now what happens if you have direct binding:
>>> device1 {
>>> 	dmas = <&dma2 1 4 0x10400 0x3>;
>>> };
>>>
>>> and via event router:
>>> device2 {
>>> 	dmas =	<&dma_router 10 0x10400 0x3>,
>>> 		<&dma_router 11 0x10400 0x3>;
>>> };
>>>
>>> device2 probes first, will get channelID 0 and 1 via 
>>> dma_get_any_slave_channel.
>>>
>>> When device1 tries to probe, the channelID 1 is already taken..
>>
>> Yes this is a flaw if we mix up bindings.
>>
>>>
>>> You need to convert all peripherals to use the event router to avoid 
>>> such a races. I have done the same for the dra7.
>>> Add the event router driver,
>>> add the event router node and convert existing users to use that
>>> when adding new devices, use the event router node.
>>>
>>> The event router's binding would have 3 parameters, it manages the 
>>> available channelIDs, like the ti-dma-crossbar does for dra7. At 
>>> allocate time it would pick an unused channelID and craft the dma-spec 
>>> with the four parameters as documented.
>>> The main DMA driver will not need any modification as everything will be 
>>> taken care of by the event router.
>>>
>>
>> I look up what has been done in ti-dma-crossbar and actually this DMAMUX driver
>> has been well inspired from ti-dma-crossbar.
>> Nonetheless I understand what you meant. The channelID doesn't come from the
>> dmaengine and a piece a code is devised to allocate those. I could copy/paste
>> such code in my side but I do believe this would be better if such information
>> would come from dmaengine instead : this is what I did but a link/callback is
>> missing to craft this info until DMA. ChannelID is computed in two places in
>> dmaemgine and in your driver. Moreover any router is going to develop its own
>> channelID allocator, info which normally comes from dmaengine.
> 
> yes and no.
> In my case on dr7 we have DMA request crossbar to support more events
> than either eDMA or sDMA could possible handle. eDMA and sDMA works in
> different ways, but the same event router driver facilitates them fine.
> In sDMA any channel can service any DMA requests, while in eDMA the
> channel number and the event numbers are matched (ch10 can service only
> event10).
> 
> Our event router driver's task is to map the incoming event number to an
> outgoing event number, which is then going to be used by the DMA driver
> as incoming event. So in the crossbar we anyways need to pick an event
> from the available list of unused ones. The DMA drivers (eDMA or sDMA)
> would just think that the request comes via normal binding and does what
> it normally does on SoC where we don't have the crossbar (OMAPs for
> sDMA, daVinci, am33/am43, k2g, etc for eDMA).
> 
> I'm not sure what your DMAMUX muxes, is it the channels or the events,
> but it only need to manage the needed, moving parts.
> 

Our DMAMUX can manage up to 255 request lines (only 128 is eventually assigned
though) onto 16 events: 8 events mapped on 1 DMA and the 8 others onto the
second DMA. Request line numbering is fixed (a peripheral DMA request is
assigned to one MUX input) and but can be routed randomly onto the any 16
channels. We use chanID to mux input on event.
chanID given by dma_get_any_slave_channe is enough in our case.

Py

>> Vinod, I can update my driver to mimic what ti-dma-crossbar did to avoid the
>> custom API. This is s rather big change to evaluate in my side though.
>> However it seems to me such info should have come from dmaengine and not from
>> driver.
>> Let me know your thought about this
>>
>>> The only gotcha is with memcpy type of transfers as they might also need 
>>> unique channelID, but not requested via the slave binding. For that I 
>>> have added properties to the event router to mask out certain channels 
>>> (and I needed to do the same for the eDMA, but it is unrelated to the 
>>> router itself).
>>>
>>>
>>> - Péter
>>>
>>
>> Py
>>
> 
> - Péter
> 

Powered by blists - more mailing lists