diff mbox

[v3,2/5] dmaengine: Add STM32 DMAMUX driver

Message ID 1499343623-5964-3-git-send-email-pierre-yves.mordret@st.com (mailing list archive)
State New, archived
Headers show

Commit Message

Pierre Yves MORDRET July 6, 2017, 12:20 p.m. UTC
This patch implements the STM32 DMAMUX driver.

The DMAMUX request multiplexer allows routing a DMA request line between
the peripherals and the DMA controllers of the product. The routing
function is ensured by a programmable multi-channel DMA request line
multiplexer. Each channel selects a unique DMA request line,
unconditionally or synchronously with events from its DMAMUX
synchronization inputs. The DMAMUX may also be used as a DMA request
generator from programmable events on its input trigger signals

Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com>
---
 Version history:
    v3:
        * change compatible to st,stm32h7-dmamux to be mode Soc specific
    v2:
        * Dynamic channelID allocation.
        * Change of_property_... by device_property.
        * New clock management.
        * DMAMUX Configuration API.
---
---
 drivers/dma/Kconfig              |   9 ++
 drivers/dma/Makefile             |   1 +
 drivers/dma/stm32-dmamux.c       | 253 +++++++++++++++++++++++++++++++++++++++
 include/linux/dma/stm32-dmamux.h |  21 ++++
 4 files changed, 284 insertions(+)
 create mode 100644 drivers/dma/stm32-dmamux.c
 create mode 100644 include/linux/dma/stm32-dmamux.h

Comments

Vinod Koul July 22, 2017, 6:51 a.m. UTC | #1
On Thu, Jul 06, 2017 at 02:20:20PM +0200, Pierre-Yves MORDRET wrote:
> +static int stm32_dmamux_probe(struct platform_device *pdev)
> +{
> +	struct device_node *node = pdev->dev.of_node;
> +	struct device_node *dma_node;
> +	struct stm32_dmamux_data *stm32_dmamux;
> +	struct resource *res;
> +	void __iomem *iomem;
> +	int i, ret;
> +
> +	if (!node)
> +		return -ENODEV;
> +
> +	stm32_dmamux = devm_kzalloc(&pdev->dev, sizeof(*stm32_dmamux),
> +				    GFP_KERNEL);
> +	if (!stm32_dmamux)
> +		return -ENOMEM;
> +
> +	dma_node = of_parse_phandle(node, "dma-masters", 0);
> +	if (!dma_node) {
> +		dev_err(&pdev->dev, "Can't get DMA master node\n");
> +		return -ENODEV;
> +	}
> +
> +	if (device_property_read_u32(&pdev->dev, "dma-channels",
> +				     &stm32_dmamux->dmamux_channels))
> +		stm32_dmamux->dmamux_channels = STM32_DMAMUX_MAX_CHANNELS;
> +
> +	if (device_property_read_u32(&pdev->dev, "dma-requests",
> +				     &stm32_dmamux->dmamux_requests))
> +		stm32_dmamux->dmamux_requests = STM32_DMAMUX_MAX_REQUESTS;

I think defaults should be warned here too

> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res)
> +		return -ENODEV;
> +
> +	iomem = devm_ioremap_resource(&pdev->dev, res);
> +	if (!iomem)
> +		return -ENOMEM;
> +
> +	spin_lock_init(&stm32_dmamux->lock);
> +
> +	stm32_dmamux->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(stm32_dmamux->clk)) {
> +		dev_info(&pdev->dev, "Missing controller clock\n");

Can you check for EPROBE_DEFER and print only for if that is not the error
otherwise we end up sapmming with defered probe issues

> +
> +#ifndef __DMA_STM32_DMAMUX_H
> +#define __DMA_STM32_DMAMUX_H
> +
> +#if defined(CONFIG_STM32_DMAMUX)
> +int stm32_dmamux_set_config(struct device *dev, void *route_data, u32 chan_id);

Why do we need a custom API in this case?
Pierre Yves MORDRET July 24, 2017, 1:55 p.m. UTC | #2
On 07/22/2017 08:51 AM, Vinod Koul wrote:
> On Thu, Jul 06, 2017 at 02:20:20PM +0200, Pierre-Yves MORDRET wrote:
>> +static int stm32_dmamux_probe(struct platform_device *pdev)
>> +{
>> +	struct device_node *node = pdev->dev.of_node;
>> +	struct device_node *dma_node;
>> +	struct stm32_dmamux_data *stm32_dmamux;
>> +	struct resource *res;
>> +	void __iomem *iomem;
>> +	int i, ret;
>> +
>> +	if (!node)
>> +		return -ENODEV;
>> +
>> +	stm32_dmamux = devm_kzalloc(&pdev->dev, sizeof(*stm32_dmamux),
>> +				    GFP_KERNEL);
>> +	if (!stm32_dmamux)
>> +		return -ENOMEM;
>> +
>> +	dma_node = of_parse_phandle(node, "dma-masters", 0);
>> +	if (!dma_node) {
>> +		dev_err(&pdev->dev, "Can't get DMA master node\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	if (device_property_read_u32(&pdev->dev, "dma-channels",
>> +				     &stm32_dmamux->dmamux_channels))
>> +		stm32_dmamux->dmamux_channels = STM32_DMAMUX_MAX_CHANNELS;
>> +
>> +	if (device_property_read_u32(&pdev->dev, "dma-requests",
>> +				     &stm32_dmamux->dmamux_requests))
>> +		stm32_dmamux->dmamux_requests = STM32_DMAMUX_MAX_REQUESTS;
> 
> I think defaults should be warned here too
> 

ok

>> +
>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	if (!res)
>> +		return -ENODEV;
>> +
>> +	iomem = devm_ioremap_resource(&pdev->dev, res);
>> +	if (!iomem)
>> +		return -ENOMEM;
>> +
>> +	spin_lock_init(&stm32_dmamux->lock);
>> +
>> +	stm32_dmamux->clk = devm_clk_get(&pdev->dev, NULL);
>> +	if (IS_ERR(stm32_dmamux->clk)) {
>> +		dev_info(&pdev->dev, "Missing controller clock\n");
> 
> Can you check for EPROBE_DEFER and print only for if that is not the error
> otherwise we end up sapmming with defered probe issues
> 

This is what you meant ?
	if (IS_ERR(stm32_dmamux->clk) != EPROBE_DEFER) {
		dev_info(&pdev->dev, "Missing controller clock\n");
		return PTR_ERR(stm32_dmamux->clk);
	}

OR

	if (IS_ERR(stm32_dmamux->clk)) {
		if (IS_ERR(stm32_dmamux->clk) != EPROBE_DEFER)
			dev_info(&pdev->dev, "Missing controller clock\n");
		return PTR_ERR(stm32_dmamux->clk);
	}

>> +
>> +#ifndef __DMA_STM32_DMAMUX_H
>> +#define __DMA_STM32_DMAMUX_H
>> +
>> +#if defined(CONFIG_STM32_DMAMUX)
>> +int stm32_dmamux_set_config(struct device *dev, void *route_data, u32 chan_id);
> 
> Why do we need a custom API in this case?
> 

This API is called by DMA when a slave is requested by client. DMA can work
without DMAMUX this API has been put in place to configure DMAMUX whether client
is requesting a DMAMUX Channel instead of a DMA one.

Thanks
Py
Vinod Koul July 26, 2017, 5:29 a.m. UTC | #3
On Mon, Jul 24, 2017 at 01:55:10PM +0000, Pierre Yves MORDRET wrote:

> >> +
> >> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >> +	if (!res)
> >> +		return -ENODEV;
> >> +
> >> +	iomem = devm_ioremap_resource(&pdev->dev, res);
> >> +	if (!iomem)
> >> +		return -ENOMEM;
> >> +
> >> +	spin_lock_init(&stm32_dmamux->lock);
> >> +
> >> +	stm32_dmamux->clk = devm_clk_get(&pdev->dev, NULL);
> >> +	if (IS_ERR(stm32_dmamux->clk)) {
> >> +		dev_info(&pdev->dev, "Missing controller clock\n");
> > 
> > Can you check for EPROBE_DEFER and print only for if that is not the error
> > otherwise we end up sapmming with defered probe issues
> > 
> 
> This is what you meant ?
> 	if (IS_ERR(stm32_dmamux->clk) != EPROBE_DEFER) {
> 		dev_info(&pdev->dev, "Missing controller clock\n");
> 		return PTR_ERR(stm32_dmamux->clk);
> 	}
> 
> OR
> 
> 	if (IS_ERR(stm32_dmamux->clk)) {
> 		if (IS_ERR(stm32_dmamux->clk) != EPROBE_DEFER)
> 			dev_info(&pdev->dev, "Missing controller clock\n");
> 		return PTR_ERR(stm32_dmamux->clk);
> 	}

This one please

> 
> >> +
> >> +#ifndef __DMA_STM32_DMAMUX_H
> >> +#define __DMA_STM32_DMAMUX_H
> >> +
> >> +#if defined(CONFIG_STM32_DMAMUX)
> >> +int stm32_dmamux_set_config(struct device *dev, void *route_data, u32 chan_id);
> > 
> > Why do we need a custom API in this case?
> > 
> 
> This API is called by DMA when a slave is requested by client. DMA can work
> without DMAMUX this API has been put in place to configure DMAMUX whether client
> is requesting a DMAMUX Channel instead of a DMA one.

You mean the dmaengine driver right?
Pierre Yves MORDRET July 26, 2017, 7:38 a.m. UTC | #4
On 07/26/2017 07:29 AM, Vinod Koul wrote:
> On Mon, Jul 24, 2017 at 01:55:10PM +0000, Pierre Yves MORDRET wrote:
> 
>>>> +
>>>> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>> +	if (!res)
>>>> +		return -ENODEV;
>>>> +
>>>> +	iomem = devm_ioremap_resource(&pdev->dev, res);
>>>> +	if (!iomem)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	spin_lock_init(&stm32_dmamux->lock);
>>>> +
>>>> +	stm32_dmamux->clk = devm_clk_get(&pdev->dev, NULL);
>>>> +	if (IS_ERR(stm32_dmamux->clk)) {
>>>> +		dev_info(&pdev->dev, "Missing controller clock\n");
>>>
>>> Can you check for EPROBE_DEFER and print only for if that is not the error
>>> otherwise we end up sapmming with defered probe issues
>>>
>>
>> This is what you meant ?
>> 	if (IS_ERR(stm32_dmamux->clk) != EPROBE_DEFER) {
>> 		dev_info(&pdev->dev, "Missing controller clock\n");
>> 		return PTR_ERR(stm32_dmamux->clk);
>> 	}
>>
>> OR
>>
>> 	if (IS_ERR(stm32_dmamux->clk)) {
>> 		if (IS_ERR(stm32_dmamux->clk) != EPROBE_DEFER)
>> 			dev_info(&pdev->dev, "Missing controller clock\n");
>> 		return PTR_ERR(stm32_dmamux->clk);
>> 	}
> 
> This one please
> 

ok

>>
>>>> +
>>>> +#ifndef __DMA_STM32_DMAMUX_H
>>>> +#define __DMA_STM32_DMAMUX_H
>>>> +
>>>> +#if defined(CONFIG_STM32_DMAMUX)
>>>> +int stm32_dmamux_set_config(struct device *dev, void *route_data, u32 chan_id);
>>>
>>> Why do we need a custom API in this case?
>>>
>>
>> This API is called by DMA when a slave is requested by client. DMA can work
>> without DMAMUX this API has been put in place to configure DMAMUX whether client
>> is requesting a DMAMUX Channel instead of a DMA one.
> 
> You mean the dmaengine driver right?
> 

Yes. The API is mainly called by "device_config" through out STM32 DMA Driver
when a router is in place for client.
Please refer to Patch 4/5 on this set.

Thanks
Py
Vinod Koul July 31, 2017, 12:31 p.m. UTC | #5
On Wed, Jul 26, 2017 at 07:38:02AM +0000, Pierre Yves MORDRET wrote:
> >>>> +
> >>>> +#ifndef __DMA_STM32_DMAMUX_H
> >>>> +#define __DMA_STM32_DMAMUX_H
> >>>> +
> >>>> +#if defined(CONFIG_STM32_DMAMUX)
> >>>> +int stm32_dmamux_set_config(struct device *dev, void *route_data, u32 chan_id);
> >>>
> >>> Why do we need a custom API in this case?
> >>>
> >>
> >> This API is called by DMA when a slave is requested by client. DMA can work
> >> without DMAMUX this API has been put in place to configure DMAMUX whether client
> >> is requesting a DMAMUX Channel instead of a DMA one.
> > 
> > You mean the dmaengine driver right?
> > 
> 
> Yes. The API is mainly called by "device_config" through out STM32 DMA Driver
> when a router is in place for client.
> Please refer to Patch 4/5 on this set.

Okay am thinking on why this can't be generic..? An optional router config
callback?
Pierre Yves MORDRET Aug. 1, 2017, 9:32 a.m. UTC | #6
On 07/31/2017 02:31 PM, Vinod Koul wrote:
> On Wed, Jul 26, 2017 at 07:38:02AM +0000, Pierre Yves MORDRET wrote:
>>>>>> +
>>>>>> +#ifndef __DMA_STM32_DMAMUX_H
>>>>>> +#define __DMA_STM32_DMAMUX_H
>>>>>> +
>>>>>> +#if defined(CONFIG_STM32_DMAMUX)
>>>>>> +int stm32_dmamux_set_config(struct device *dev, void *route_data, u32 chan_id);
>>>>>
>>>>> Why do we need a custom API in this case?
>>>>>
>>>>
>>>> This API is called by DMA when a slave is requested by client. DMA can work
>>>> without DMAMUX this API has been put in place to configure DMAMUX whether client
>>>> is requesting a DMAMUX Channel instead of a DMA one.
>>>
>>> You mean the dmaengine driver right?
>>>
>>
>> Yes. The API is mainly called by "device_config" through out STM32 DMA Driver
>> when a router is in place for client.
>> Please refer to Patch 4/5 on this set.
> 
> Okay am thinking on why this can't be generic..? An optional router config
> callback?
> 

I would have liked to answer there is a callback within engine but unfortunately
I didn't figure out one when I did my router's development. I've looked once
more but again I can't find how to map chanID and request line without custom API.
Vinod Koul Aug. 2, 2017, 4:55 a.m. UTC | #7
On Tue, Aug 01, 2017 at 09:32:50AM +0000, Pierre Yves MORDRET wrote:
> 
> 
> On 07/31/2017 02:31 PM, Vinod Koul wrote:
> > On Wed, Jul 26, 2017 at 07:38:02AM +0000, Pierre Yves MORDRET wrote:
> >>>>>> +
> >>>>>> +#ifndef __DMA_STM32_DMAMUX_H
> >>>>>> +#define __DMA_STM32_DMAMUX_H
> >>>>>> +
> >>>>>> +#if defined(CONFIG_STM32_DMAMUX)
> >>>>>> +int stm32_dmamux_set_config(struct device *dev, void *route_data, u32 chan_id);
> >>>>>
> >>>>> Why do we need a custom API in this case?
> >>>>>
> >>>>
> >>>> This API is called by DMA when a slave is requested by client. DMA can work
> >>>> without DMAMUX this API has been put in place to configure DMAMUX whether client
> >>>> is requesting a DMAMUX Channel instead of a DMA one.
> >>>
> >>> You mean the dmaengine driver right?
> >>>
> >>
> >> Yes. The API is mainly called by "device_config" through out STM32 DMA Driver
> >> when a router is in place for client.
> >> Please refer to Patch 4/5 on this set.
> > 
> > Okay am thinking on why this can't be generic..? An optional router config
> > callback?
> > 
> 
> I would have liked to answer there is a callback within engine but unfortunately
> I didn't figure out one when I did my router's development. I've looked once
> more but again I can't find how to map chanID and request line without custom API.

Yes there is no callback for routers but we can add a generic callback
here to be used. I added Peter for his comments, isn't that something they
need too?
Peter Ujfalusi Aug. 2, 2017, 9:19 a.m. UTC | #8
On 2017-08-02 07:55, Vinod Koul wrote:
> On Tue, Aug 01, 2017 at 09:32:50AM +0000, Pierre Yves MORDRET wrote:
>>
>>
>> On 07/31/2017 02:31 PM, Vinod Koul wrote:
>>> On Wed, Jul 26, 2017 at 07:38:02AM +0000, Pierre Yves MORDRET wrote:
>>>>>>>> +
>>>>>>>> +#ifndef __DMA_STM32_DMAMUX_H
>>>>>>>> +#define __DMA_STM32_DMAMUX_H
>>>>>>>> +
>>>>>>>> +#if defined(CONFIG_STM32_DMAMUX)
>>>>>>>> +int stm32_dmamux_set_config(struct device *dev, void *route_data, u32 chan_id);
>>>>>>>
>>>>>>> Why do we need a custom API in this case?
>>>>>>>
>>>>>>
>>>>>> This API is called by DMA when a slave is requested by client. DMA can work
>>>>>> without DMAMUX this API has been put in place to configure DMAMUX whether client
>>>>>> is requesting a DMAMUX Channel instead of a DMA one.
>>>>>
>>>>> You mean the dmaengine driver right?
>>>>>
>>>>
>>>> Yes. The API is mainly called by "device_config" through out STM32 DMA Driver
>>>> when a router is in place for client.
>>>> Please refer to Patch 4/5 on this set.
>>>
>>> Okay am thinking on why this can't be generic..? An optional router config
>>> callback?
>>>
>>
>> I would have liked to answer there is a callback within engine but unfortunately
>> I didn't figure out one when I did my router's development. I've looked once
>> more but again I can't find how to map chanID and request line without custom API.
>
> Yes there is no callback for routers but we can add a generic callback
> here to be used. I added Peter for his comments, isn't that something they
> need too?

The event router via of_dma_router_register() should be capable of 
handling different type of muxers, like the ti-dma-crossbar.c is doing 
for two different type of event crossbars.

Basically with the of_dma_route_allocate you craft a dma_spec which can 
be understood by the dma-master pointed form the router's node.
You do the configuration of the mux in this function, craft the dma_spec 
and that's it. In DT the peripherals are using the router's node for DMA 
binding and everything is transparent for them.

Note: The use of am335x xbar in the ti-dma-crossbar is optional and only 
needed when we need to have different event than the default for a 
specific dma request line.

If you normally use the DMA like this:
dmas = <&edma 129 1>, <&ddma_xbar 128 1>;
dma-names = "tx", "rx";

If you have DMA event router/mux, then depending on how it works you 
might have different number of parameters. In my case the DRA7 crossbar 
does not need extra parameter, so to get it in use:
dmas = <&edma_xbar 129 1>, <&edma_xbar 128 1>;
dma-names = "tx", "rx";

The router driver will rewrite the dma_spec and replace the 129/128 and 
pass something different to the dma-master (dynamic event mapping).

On am335x we have different xbar type so there:

dmas = <&edma_xbar 12 0 1>, <&edma_xbar 13 0 2>;

Out from this the router driver will create a spec equivalent to
dmas = <&edma_xbar 12 0>, <&edma_xbar 13 0>;

But it will change the xbar that DMA request 12/13 will not have the 
default event routed to.

I believe that the dma_router infra we have in dmaengine can cover most 
if not all use cases...

- Péter
Pierre Yves MORDRET Aug. 2, 2017, 1:11 p.m. UTC | #9
On 08/02/2017 11:19 AM, Peter Ujfalusi wrote:
> 

> 

> On 2017-08-02 07:55, Vinod Koul wrote:

>> On Tue, Aug 01, 2017 at 09:32:50AM +0000, Pierre Yves MORDRET wrote:

>>>

>>>

>>> On 07/31/2017 02:31 PM, Vinod Koul wrote:

>>>> On Wed, Jul 26, 2017 at 07:38:02AM +0000, Pierre Yves MORDRET wrote:

>>>>>>>>> +

>>>>>>>>> +#ifndef __DMA_STM32_DMAMUX_H

>>>>>>>>> +#define __DMA_STM32_DMAMUX_H

>>>>>>>>> +

>>>>>>>>> +#if defined(CONFIG_STM32_DMAMUX)

>>>>>>>>> +int stm32_dmamux_set_config(struct device *dev, void *route_data, u32 chan_id);

>>>>>>>>

>>>>>>>> Why do we need a custom API in this case?

>>>>>>>>

>>>>>>>

>>>>>>> This API is called by DMA when a slave is requested by client. DMA can work

>>>>>>> without DMAMUX this API has been put in place to configure DMAMUX whether client

>>>>>>> is requesting a DMAMUX Channel instead of a DMA one.

>>>>>>

>>>>>> You mean the dmaengine driver right?

>>>>>>

>>>>>

>>>>> Yes. The API is mainly called by "device_config" through out STM32 DMA Driver

>>>>> when a router is in place for client.

>>>>> Please refer to Patch 4/5 on this set.

>>>>

>>>> Okay am thinking on why this can't be generic..? An optional router config

>>>> callback?

>>>>

>>>

>>> I would have liked to answer there is a callback within engine but unfortunately

>>> I didn't figure out one when I did my router's development. I've looked once

>>> more but again I can't find how to map chanID and request line without custom API.

>>

>> Yes there is no callback for routers but we can add a generic callback

>> here to be used. I added Peter for his comments, isn't that something they

>> need too?

> 

> The event router via of_dma_router_register() should be capable of 

> handling different type of muxers, like the ti-dma-crossbar.c is doing 

> for two different type of event crossbars.

> 

> Basically with the of_dma_route_allocate you craft a dma_spec which can 

> be understood by the dma-master pointed form the router's node.

> You do the configuration of the mux in this function, craft the dma_spec 

> and that's it. In DT the peripherals are using the router's node for DMA 

> binding and everything is transparent for them.

> 

> Note: The use of am335x xbar in the ti-dma-crossbar is optional and only 

> needed when we need to have different event than the default for a 

> specific dma request line.

> 

> If you normally use the DMA like this:

> dmas = <&edma 129 1>, <&ddma_xbar 128 1>;

> dma-names = "tx", "rx";

> 

> If you have DMA event router/mux, then depending on how it works you 

> might have different number of parameters. In my case the DRA7 crossbar 

> does not need extra parameter, so to get it in use:

> dmas = <&edma_xbar 129 1>, <&edma_xbar 128 1>;

> dma-names = "tx", "rx";

> 

> The router driver will rewrite the dma_spec and replace the 129/128 and 

> pass something different to the dma-master (dynamic event mapping).

> 

> On am335x we have different xbar type so there:

> 

> dmas = <&edma_xbar 12 0 1>, <&edma_xbar 13 0 2>;

> 

> Out from this the router driver will create a spec equivalent to

> dmas = <&edma_xbar 12 0>, <&edma_xbar 13 0>;

> 

> But it will change the xbar that DMA request 12/13 will not have the 

> default event routed to.

> 

> I believe that the dma_router infra we have in dmaengine can cover most 

> if not all use cases...

> 

> - Péter

> 


Our SoC works with or without DMAMUX. Both binding is allowed. Using only DMA a
ChannelId and request line is part of the binding. Using DMAMUx now the request
line is coming from dma_spec forwards to dma-master as well explained by Peter.
However ChannelID is now given by dma_get_any_slave_channel instead of bindings.
DMAMUX driver has to be aware of this ID to route request line to out DMA
channel. This channel id information is carried on until DMAMUX through
dmaengine_slave_config with a custom API.
Hope it clarifies the need.
Peter Ujfalusi Aug. 2, 2017, 2:09 p.m. UTC | #10

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

On 2017-08-02 16:11, Pierre Yves MORDRET wrote:
> Our SoC works with or without DMAMUX. Both binding is allowed. Using only DMA a
> ChannelId and request line is part of the binding.

In our case the am335x's eDMA can work with or without the router, we 
only use the router node if we need none default event for a given DMA 
request line.

> Using DMAMUx now the request
> line is coming from dma_spec forwards to dma-master as well explained by Peter.
> However ChannelID is now given by dma_get_any_slave_channel instead of bindings.
> DMAMUX driver has to be aware of this ID to route request line to out DMA
> channel. This channel id information is carried on until DMAMUX through
> dmaengine_slave_config with a custom API.
> Hope it clarifies the need.

I see, this is not much different then what we face with our dra7 
devices. In theory we could use direct DMA binding to the DMA controller 
itself, but some requests would not be reachable, so we always use the 
router's node for DMA on dra7 family.

Basically the router would manage the ChannelID and create 
'st,stm32-dma' compatible dma_spec (the four parameters).
Afaik you could have 3 parameters for the router and create a four 
parameter dma_spec, where the ChannelID is dynamically allocated.
But you need to convert all peripherals to use the router's node for the 
DMA.

- Péter
Pierre Yves MORDRET Aug. 2, 2017, 2:28 p.m. UTC | #11
On 08/02/2017 04:09 PM, Peter Ujfalusi wrote:
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 
> On 2017-08-02 16:11, Pierre Yves MORDRET wrote:
>> Our SoC works with or without DMAMUX. Both binding is allowed. Using only DMA a
>> ChannelId and request line is part of the binding.
> 
> In our case the am335x's eDMA can work with or without the router, we 
> only use the router node if we need none default event for a given DMA 
> request line.
> 
>> Using DMAMUx now the request
>> line is coming from dma_spec forwards to dma-master as well explained by Peter.
>> However ChannelID is now given by dma_get_any_slave_channel instead of bindings.
>> DMAMUX driver has to be aware of this ID to route request line to out DMA
>> channel. This channel id information is carried on until DMAMUX through
>> dmaengine_slave_config with a custom API.
>> Hope it clarifies the need.
> 
> I see, this is not much different then what we face with our dra7 
> devices. In theory we could use direct DMA binding to the DMA controller 
> itself, but some requests would not be reachable, so we always use the 
> router's node for DMA on dra7 family.
> 
> Basically the router would manage the ChannelID and create 
> 'st,stm32-dma' compatible dma_spec (the four parameters).
> Afaik you could have 3 parameters for the router and create a four 
> parameter dma_spec, where the ChannelID is dynamically allocated.

Correct router needs 3 parameters and among those 2 are forwarded though out
dma_spec. But when you say "ChannelID is dynamically allocated" you mean
dma_get_any_slave_channel ? If yes I can use the already existing bindings to
carry the channelID to DMA. No changes need to peripheral...

> But you need to convert all peripherals to use the router's node for the 
> DMA.
> 
> - Péter
> 

Py
Peter Ujfalusi Aug. 3, 2017, 6:42 a.m. UTC | #12
our mail server started to mangle outgoing mails, sorry for that, we are 
trying to resolve that...


Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

On 2017-08-02 17:28, Pierre Yves MORDRET wrote:
> Correct router needs 3 parameters and among those 2 are forwarded though out
> dma_spec. But when you say "ChannelID is dynamically allocated" you mean
> dma_get_any_slave_channel ? If yes I can use the already existing bindings to
> carry the channelID to DMA. No changes need to peripheral...

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?
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?
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.

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..

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.

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
Pierre Yves MORDRET Aug. 3, 2017, 9 a.m. UTC | #13
On 08/03/2017 08:42 AM, Peter Ujfalusi wrote:
> our mail server started to mangle outgoing mails, sorry for that, we are 
> trying to resolve that...

No problem ;)

> 
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 
> On 2017-08-02 17:28, Pierre Yves MORDRET wrote:
>> Correct router needs 3 parameters and among those 2 are forwarded though out
>> dma_spec. But when you say "ChannelID is dynamically allocated" you mean
>> dma_get_any_slave_channel ? If yes I can use the already existing bindings to
>> carry the channelID to DMA. No changes need to peripheral...
> 
> 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.

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
Peter Ujfalusi Aug. 3, 2017, 9:48 a.m. UTC | #14

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.

> 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
Pierre Yves MORDRET Aug. 4, 2017, 12:50 p.m. UTC | #15
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
>
Peter Ujfalusi Aug. 4, 2017, 2:21 p.m. UTC | #16
On 08/04/2017 03:50 PM, Pierre Yves MORDRET wrote:
> 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.

I would think that if you have in the router node:
dma-masters = <&dma1>, <&dma2>;

and request a DMA via the router:
dmas = <&dma_router req_in param1 param2>;

then the router driver would decide which dma-master it is going to assign the
given request line and craft the dma-spec based on this decision. This
requires no callback to the router from the DMA master driver at all.

The idea of the dma event router is to be transparent for the DMA clients
(peripherals needing DMA channel) and for the DMA drivers as well. Neither
should know that the events are muxed as it does not really matter for them.
Pierre Yves MORDRET Aug. 21, 2017, 9:34 a.m. UTC | #17
On 08/04/2017 04:21 PM, Peter Ujfalusi wrote:
> On 08/04/2017 03:50 PM, Pierre Yves MORDRET wrote:
>> 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.
> 
> I would think that if you have in the router node:
> dma-masters = <&dma1>, <&dma2>;
> 
> and request a DMA via the router:
> dmas = <&dma_router req_in param1 param2>;
> 
> then the router driver would decide which dma-master it is going to assign the
> given request line and craft the dma-spec based on this decision. This
> requires no callback to the router from the DMA master driver at all.
> 
> The idea of the dma event router is to be transparent for the DMA clients
> (peripherals needing DMA channel) and for the DMA drivers as well. Neither
> should know that the events are muxed as it does not really matter for them.
>

OK. I will redesign my driver to take into account this idea.

I believe I should get rid of my custom API in DMA for channelID as well. Please
confirm. Not very clear for me whether I can keep it or not.

Regards
PY
Peter Ujfalusi Aug. 24, 2017, 5:47 a.m. UTC | #18
On 2017-08-21 12:34, Pierre Yves MORDRET wrote:
> OK. I will redesign my driver to take into account this idea.
> 
> I believe I should get rid of my custom API in DMA for channelID as well. Please
> confirm. Not very clear for me whether I can keep it or not.

Yes, you should be able to get rid of any custom API.
The DMA event router should be 'invisible' to both the DMA driver itself
and for the DMA users as well. If you end up still needing custom API
(which I doubt) then it is better to extend the core support to cover
that in a generic way since it is likely that other might have similar
needs.

You need to identify what you need to manage with the DMA router, again
in my cases:
am335x/am437x: The DMA channel is fixed, but the DMA event to be handled
by the channel (DMA event muxer) is selected by the router.
dra7x: The DMA event is the fixed part and I needed to translate that to
local eDMA/sDMA events and in eDMA I needed to pick a channel based on
the translated event.

The same router code works for eDMA and sDMA in dra7, while the two DMA
engines are different in their internal works.

At the end of the day I only needed to plug the DMA event routers and
there is no change in the DMA drivers at all.

I'm happy to answer any questions you might have, if I can.

- Péter
Pierre Yves MORDRET Aug. 24, 2017, 1:03 p.m. UTC | #19
On 08/24/2017 07:47 AM, Peter Ujfalusi wrote:
> 
> 
> On 2017-08-21 12:34, Pierre Yves MORDRET wrote:
>> OK. I will redesign my driver to take into account this idea.
>>
>> I believe I should get rid of my custom API in DMA for channelID as well. Please
>> confirm. Not very clear for me whether I can keep it or not.
> 
> Yes, you should be able to get rid of any custom API.
> The DMA event router should be 'invisible' to both the DMA driver itself
> and for the DMA users as well. If you end up still needing custom API
> (which I doubt) then it is better to extend the core support to cover
> that in a generic way since it is likely that other might have similar
> needs.
> 

Will see that later on :)

> You need to identify what you need to manage with the DMA router, again
> in my cases:
> am335x/am437x: The DMA channel is fixed, but the DMA event to be handled
> by the channel (DMA event muxer) is selected by the router.
> dra7x: The DMA event is the fixed part and I needed to translate that to
> local eDMA/sDMA events and in eDMA I needed to pick a channel based on
> the translated event.
> 
> The same router code works for eDMA and sDMA in dra7, while the two DMA
> engines are different in their internal works.
> 
> At the end of the day I only needed to plug the DMA event routers and
> there is no change in the DMA drivers at all.
> 

Please tell me whether I'm wrong but for am335x/am437x both DMA Channels and
events are given by DT. I believe IP Spec provides the mapping for the channel
(this is what you call fixed channel) and DMA router event is selected randomly
within the DT.
As for dra7 events comes from DT but channel is selected though out local
algorithm. IP Spec defines DMA event muxer mapping.

At my opinion my router is more closed to dra7. IP Spec defines event mapping.
Nonetheless the DMA has a fixed allocated mapping. Using DMA alone DT has to
provide channel number. In router mode this number doesn't matter since router
makes the routing from fixed event to channel. However router needs to know
which channel will be assign to event: any random channel is allowed.

I'm pretty sure I can mimic what has been for DRA7 related to DMA Channel
allocation however it seems to be this is aside DMA engine. This kind of
implementation forbid the use of DMA and DMA router at the same time : I
remember you already raise the point. If a DMA channel is requested DMA router
is not aware of this allocation. This is the idea of my custom API which relies
on get_any_slave_channel().
Using DT to assign channel seems not a good idea either as I lost router's benefice.

BTW I need the channel ID within router.
Looking at core or of_dma_router_xlate() I don't really know how to do it a
generic manner.

Ideas are welcomed

> I'm happy to answer any questions you might have, if I can.
> 

You will be happy then.

> - Péter
> 

Thanks
Py
Peter Ujfalusi Aug. 28, 2017, 11:48 a.m. UTC | #20

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

On 2017-08-24 16:03, Pierre Yves MORDRET wrote:
> Please tell me whether I'm wrong but for am335x/am437x both DMA Channels and
> events are given by DT. I believe IP Spec provides the mapping for the channel
> (this is what you call fixed channel) and DMA router event is selected randomly
> within the DT.

In our eDMA the channel number and the DMA event number is locked
together. Event 17 can only be serviced by eDMA channel 17. The
am335x/am437x DMA crossbar is implemented on every DMA event lane and by
changing it you can select different event to be routed to the given
event number. The eDMA can handle up to 64 unique DMA events, however
for each of this 64 events we have a crossbar which allows to select
from up to 63 other sources. I can not allocate the eDMA channel in the
crossbar as I can not be sure that I will not step on some other
driver's toe and take a channel (event) which someone want to use with
it's default purpose. So the binding includes the channel number and the
crossbar mapped event ID.

Oh, and this eDMA limitation is the main source of my hassle with the
mem-to-mem channel: in DT we must tell what channels can be used for
memcpy - the ones that are not used on the board (usually we pick i2c
since we don't support DMA with that).

> As for dra7 events comes from DT but channel is selected though out local
> algorithm. IP Spec defines DMA event muxer mapping.

Here it is again a different story. The crossbar in dra7 can handle 256
incoming events and it can map any of these events to the 64 eDMA event
number (or channel) or if it is front of the sDMA we can select the sDMA
event number from the 128 it can handle.

Here we only use the event number in the crossbar node as we will pick
the eDMA event, which is going to be correspond to the eDMA channel.

> At my opinion my router is more closed to dra7. IP Spec defines event mapping.
> Nonetheless the DMA has a fixed allocated mapping. Using DMA alone DT has to
> provide channel number. In router mode this number doesn't matter since router
> makes the routing from fixed event to channel. However router needs to know
> which channel will be assign to event: any random channel is allowed.

In dra7 both eDMA and sDMA have pre-allocated channel mapping. Before my
DMA event router support we had no way to use non default mapped events
(we did had DMA support fro HSMMC and probably other stuff with direct
DMA node).
When I had the DMA router, first I added the node for it, then I have
converted all existing nodes to use the DMA event router node for the
DMA binding. After that I could add the nodes for the peripherals
needing non default mapped DMA requests.
We don't allow direct DMA binding for dra7 class of devices at all,
everything needs to go via the xbars.

I would not bother with mixed use case here, I would just make it
mandatory to go via the router.

Again, with the eDMA I needed to 'mask' out the events/channels which we
want to use for memcpy. Define the memcpy channels in the eDMA node and
mask the same events in the crossbar as well.

> I'm pretty sure I can mimic what has been for DRA7 related to DMA Channel
> allocation however it seems to be this is aside DMA engine. This kind of
> implementation forbid the use of DMA and DMA router at the same time : I
> remember you already raise the point. If a DMA channel is requested DMA router
> is not aware of this allocation. This is the idea of my custom API which relies
> on get_any_slave_channel().

I think you must not allow mixed use for the slave channels. The none
slave (memcpy) is still a pain, but you have managed it somehow in the past.

> Using DT to assign channel seems not a good idea either as I lost router's benefice.
> 
> BTW I need the channel ID within router.

Your use case is pretty close to the dra7 one we have.

> Looking at core or of_dma_router_xlate() I don't really know how to do it a
> generic manner.

You construct the dmaspec for the DMA, which will include the channel
number you have mapped the event to. If the DMA driver needs the channel
number _and_ the event number then both, but based on your description
your DMA engine itself is close to the eDMA of ours than the sDMA.

- Péter
Pierre Yves MORDRET Aug. 30, 2017, 8:02 a.m. UTC | #21
On 08/28/2017 01:48 PM, Peter Ujfalusi wrote:
> 
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 

Thanks for your invaluable inputs. This is more clear now and revise my driver

> 
> I would not bother with mixed use case here, I would just make it
> mandatory to go via the router.
> 

I couldn't agree more. But it seems to me there is a flaw here. This should be
possible though. I will likely look at that later on.

A new revision of this driver will come soon ;)

> 
> - Péter
> 
Py
diff mbox

Patch

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index fa8f9c0..34d9088 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -477,6 +477,15 @@  config STM32_DMA
 	  If you have a board based on such a MCU and wish to use DMA say Y
 	  here.
 
+config STM32_DMAMUX
+	bool "STMicroelectronics STM32 dma multiplexer support"
+	depends on STM32_DMA || COMPILE_TEST
+	help
+	  Enable support for the on-chip DMA multiplexer on STMicroelectronics
+	  STM32 MCUs.
+	  If you have a board based on such a MCU and wish to use DMAMUX say Y
+	  here.
+
 config S3C24XX_DMAC
 	bool "Samsung S3C24XX DMA support"
 	depends on ARCH_S3C24XX || COMPILE_TEST
diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index d12ab29..96bd47e 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -58,6 +58,7 @@  obj-$(CONFIG_RENESAS_DMA) += sh/
 obj-$(CONFIG_SIRF_DMA) += sirf-dma.o
 obj-$(CONFIG_STE_DMA40) += ste_dma40.o ste_dma40_ll.o
 obj-$(CONFIG_STM32_DMA) += stm32-dma.o
+obj-$(CONFIG_STM32_DMAMUX) += stm32-dmamux.o
 obj-$(CONFIG_S3C24XX_DMAC) += s3c24xx-dma.o
 obj-$(CONFIG_TXX9_DMAC) += txx9dmac.o
 obj-$(CONFIG_TEGRA20_APB_DMA) += tegra20-apb-dma.o
diff --git a/drivers/dma/stm32-dmamux.c b/drivers/dma/stm32-dmamux.c
new file mode 100644
index 0000000..d2756d5
--- /dev/null
+++ b/drivers/dma/stm32-dmamux.c
@@ -0,0 +1,253 @@ 
+/*
+ * DMA Router driver for STM32 DMA MUX
+ *
+ * Copyright (C) 2017 M'Boumba Cedric Madianga <cedric.madianga@gmail.com>
+ *
+ * Based on TI DMA Crossbar driver
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/of_device.h>
+#include <linux/of_dma.h>
+#include <linux/reset.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/dma/stm32-dmamux.h>
+
+#define STM32_DMAMUX_CCR(x)		(0x4 * (x))
+#define STM32_DMAMUX_MAX_CHANNELS	32
+#define STM32_DMAMUX_MAX_REQUESTS	255
+
+struct stm32_dmamux {
+	u32 request;
+	u32 chan_id;
+	bool busy;
+};
+
+struct stm32_dmamux_data {
+	struct dma_router dmarouter;
+	struct clk *clk;
+	struct reset_control *rst;
+	void __iomem *iomem;
+	u32 dmamux_requests; /* number of DMA requests connected to DMAMUX */
+	u32 dmamux_channels; /* Number of DMA channels supported */
+	spinlock_t lock; /* Protects register access */
+};
+
+static inline u32 stm32_dmamux_read(void __iomem *iomem, u32 reg)
+{
+	return readl_relaxed(iomem + reg);
+}
+
+static inline void stm32_dmamux_write(void __iomem *iomem, u32 reg, u32 val)
+{
+	writel_relaxed(val, iomem + reg);
+}
+
+int stm32_dmamux_set_config(struct device *dev, void *route_data, u32 chan_id)
+{
+	struct stm32_dmamux_data *dmamux = dev_get_drvdata(dev);
+	struct stm32_dmamux *mux = route_data;
+	u32 request = mux->request;
+	unsigned long flags;
+	int ret;
+
+	if (chan_id >= dmamux->dmamux_channels) {
+		dev_err(dev, "invalid channel id\n");
+		return -EINVAL;
+	}
+
+	/* Set dma request */
+	spin_lock_irqsave(&dmamux->lock, flags);
+	if (!IS_ERR(dmamux->clk)) {
+		ret = clk_enable(dmamux->clk);
+		if (ret < 0) {
+			spin_unlock_irqrestore(&dmamux->lock, flags);
+			dev_err(dev, "clk_prep_enable issue: %d\n", ret);
+			return ret;
+		}
+	}
+
+	stm32_dmamux_write(dmamux->iomem, STM32_DMAMUX_CCR(chan_id), request);
+
+	mux->chan_id = chan_id;
+	mux->busy = true;
+	spin_unlock_irqrestore(&dmamux->lock, flags);
+
+	dev_dbg(dev, "Mapping dma-router%dchan%d to request%d\n", dev->id,
+		mux->chan_id, mux->request);
+
+	return 0;
+}
+
+static void stm32_dmamux_free(struct device *dev, void *route_data)
+{
+	struct stm32_dmamux_data *dmamux = dev_get_drvdata(dev);
+	struct stm32_dmamux *mux = route_data;
+	unsigned long flags;
+
+	/* Clear dma request */
+	spin_lock_irqsave(&dmamux->lock, flags);
+	if (!mux->busy) {
+		spin_unlock_irqrestore(&dmamux->lock, flags);
+		goto end;
+	}
+
+	stm32_dmamux_write(dmamux->iomem, STM32_DMAMUX_CCR(mux->chan_id), 0);
+	if (!IS_ERR(dmamux->clk))
+		clk_disable(dmamux->clk);
+	spin_unlock_irqrestore(&dmamux->lock, flags);
+
+	dev_dbg(dev, "Unmapping dma-router%dchan%d (was routed to request%d)\n",
+		dev->id, mux->chan_id, mux->request);
+
+end:
+	kfree(mux);
+}
+
+static void *stm32_dmamux_route_allocate(struct of_phandle_args *dma_spec,
+					 struct of_dma *ofdma)
+{
+	struct platform_device *pdev = of_find_device_by_node(ofdma->of_node);
+	struct stm32_dmamux_data *dmamux = platform_get_drvdata(pdev);
+	struct stm32_dmamux *mux;
+
+	if (dma_spec->args_count != 3) {
+		dev_err(&pdev->dev, "invalid number of dma mux args\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (dma_spec->args[0] > dmamux->dmamux_requests) {
+		dev_err(&pdev->dev, "invalid mux request number: %d\n",
+			dma_spec->args[0]);
+		return ERR_PTR(-EINVAL);
+	}
+
+	/* The of_node_put() will be done in of_dma_router_xlate function */
+	dma_spec->np = of_parse_phandle(ofdma->of_node, "dma-masters", 0);
+	if (!dma_spec->np) {
+		dev_err(&pdev->dev, "can't get dma master\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	mux = kzalloc(sizeof(*mux), GFP_KERNEL);
+	if (!mux) {
+		of_node_put(dma_spec->np);
+		return ERR_PTR(-ENOMEM);
+	}
+	mux->request = dma_spec->args[0];
+
+	dma_spec->args[3] = dma_spec->args[2];
+	dma_spec->args[2] = dma_spec->args[1];
+	dma_spec->args[1] = 0;
+	dma_spec->args[0] = 0;
+	dma_spec->args_count = 4;
+
+	return mux;
+}
+
+static int stm32_dmamux_probe(struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct device_node *dma_node;
+	struct stm32_dmamux_data *stm32_dmamux;
+	struct resource *res;
+	void __iomem *iomem;
+	int i, ret;
+
+	if (!node)
+		return -ENODEV;
+
+	stm32_dmamux = devm_kzalloc(&pdev->dev, sizeof(*stm32_dmamux),
+				    GFP_KERNEL);
+	if (!stm32_dmamux)
+		return -ENOMEM;
+
+	dma_node = of_parse_phandle(node, "dma-masters", 0);
+	if (!dma_node) {
+		dev_err(&pdev->dev, "Can't get DMA master node\n");
+		return -ENODEV;
+	}
+
+	if (device_property_read_u32(&pdev->dev, "dma-channels",
+				     &stm32_dmamux->dmamux_channels))
+		stm32_dmamux->dmamux_channels = STM32_DMAMUX_MAX_CHANNELS;
+
+	if (device_property_read_u32(&pdev->dev, "dma-requests",
+				     &stm32_dmamux->dmamux_requests))
+		stm32_dmamux->dmamux_requests = STM32_DMAMUX_MAX_REQUESTS;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
+
+	iomem = devm_ioremap_resource(&pdev->dev, res);
+	if (!iomem)
+		return -ENOMEM;
+
+	spin_lock_init(&stm32_dmamux->lock);
+
+	stm32_dmamux->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(stm32_dmamux->clk)) {
+		dev_info(&pdev->dev, "Missing controller clock\n");
+		return PTR_ERR(stm32_dmamux->clk);
+	}
+
+	stm32_dmamux->rst = devm_reset_control_get(&pdev->dev, NULL);
+	if (!IS_ERR(stm32_dmamux->rst)) {
+		reset_control_assert(stm32_dmamux->rst);
+		udelay(2);
+		reset_control_deassert(stm32_dmamux->rst);
+	}
+
+	stm32_dmamux->iomem = iomem;
+	stm32_dmamux->dmarouter.dev = &pdev->dev;
+	stm32_dmamux->dmarouter.route_free = stm32_dmamux_free;
+
+	platform_set_drvdata(pdev, stm32_dmamux);
+
+	if (!IS_ERR(stm32_dmamux->clk)) {
+		ret = clk_prepare_enable(stm32_dmamux->clk);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "clk_prep_enable issue: %d\n", ret);
+			return ret;
+		}
+	}
+
+	/* Reset the dmamux */
+	for (i = 0; i < stm32_dmamux->dmamux_channels; i++)
+		stm32_dmamux_write(stm32_dmamux->iomem, STM32_DMAMUX_CCR(i), 0);
+
+	if (!IS_ERR(stm32_dmamux->clk))
+		clk_disable(stm32_dmamux->clk);
+
+	return of_dma_router_register(node, stm32_dmamux_route_allocate,
+				     &stm32_dmamux->dmarouter);
+}
+
+static const struct of_device_id stm32_dmamux_match[] = {
+	{ .compatible = "st,stm32h7-dmamux" },
+	{},
+};
+
+static struct platform_driver stm32_dmamux_driver = {
+	.probe	= stm32_dmamux_probe,
+	.driver = {
+		.name = "stm32-dmamux",
+		.of_match_table = stm32_dmamux_match,
+	},
+};
+
+static int __init stm32_dmamux_init(void)
+{
+	return platform_driver_register(&stm32_dmamux_driver);
+}
+arch_initcall(stm32_dmamux_init);
diff --git a/include/linux/dma/stm32-dmamux.h b/include/linux/dma/stm32-dmamux.h
new file mode 100644
index 0000000..3a4eae1
--- /dev/null
+++ b/include/linux/dma/stm32-dmamux.h
@@ -0,0 +1,21 @@ 
+/*
+ * stm32-dmamux.h
+ *
+ * Copyright (C) M'Boumba Cedric Madianga 2017
+ * Author:  M'Boumba Cedric Madianga <cedric.madianga@gmail.com>
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+
+#ifndef __DMA_STM32_DMAMUX_H
+#define __DMA_STM32_DMAMUX_H
+
+#if defined(CONFIG_STM32_DMAMUX)
+int stm32_dmamux_set_config(struct device *dev, void *route_data, u32 chan_id);
+#else
+int stm32_dmamux_set_config(struct device *dev, void *route_data, u32 chan_id)
+{
+	return -ENODEV;
+}
+#endif /* CONFIG_STM32_DMAMUX */
+
+#endif /* __DMA_STM32_DMAMUX_H */