Hi,
Vinod: is it OK if I send the Documnetation/dmanegine/ update a bit later when
we agree on what form it should be?
Changes since v3:
- Comments from Russell:
- Warnings removed in case of non DT boot when taking the DMA request number
- Reduced the number of channels presented to DMAengine
- removed the dma_sig parameter from omap_dma_chan_init()
- Comments from Arnd:
- dma-device property renamed to dma-masters
- Allow list of phandes in dma-masters
Changes since v2:
- not using regmap for the TI crossbar driver.
Changes since v1:
- Comments from Russell King and Paul Bolle addressed:
- Use the added defined in the omap-dma changes
- MODULE_* removed from the ti-dma-crossbar driver.
- DMA router documentation: do not limit the #dma-cells to be the same as the
dma controller's #dma-cells. It might be possible to have a router which needs
more information than the DMA controller (direct request pairing for example)
- Use defines in the ti-dma-crossbar driver
- Binding document for the ti-dma-crossbar driver
Intro mail from v1:
The series adds support for DMA router type of devices. They are used in SoCs
which has more peripherals with DMA request lines than the DMA controller can
handle.
The router itself is not part of the DMA controller and it's operation should be
transparent (as it is in the HW) for the SW stack.
This series takes into accound the comments Sricharan received for his version
of the crossbar driver:
https://lkml.org/lkml/2014/3/7/199
This implementation is not tied to any DMA driver so it is possible to use the
framework by other vendors, also ACPI version of binding can be easy enough to
be added.
Regards,
Peter
---
Peter Ujfalusi (8):
dmaengine: of_dma: Support for DMA routers
Documentation: devicetree: dma: Binding documentation for TI DMA
crossbar
dmaengine: Add driver for TI DMA crossbar on DRA7x
dmaengine: omap-dma: Use defines for dma channels and request count
dmaengine: omap-dma: Take DMA request number from DT if it is
available
dmaengine: omap-dma: Remove mapping between virtual channels and
requests
dmaengine: omap-dma: Reduce the number of virtual channels
ARM: DTS: dra7x: Integrate sDMA crossbar
Documentation/devicetree/bindings/dma/dma.txt | 28 ++++
.../devicetree/bindings/dma/ti-dma-crossbar.txt | 52 ++++++
arch/arm/boot/dts/dra7.dtsi | 57 ++++---
drivers/dma/Kconfig | 4 +
drivers/dma/Makefile | 1 +
drivers/dma/dmaengine.c | 7 +
drivers/dma/of-dma.c | 86 ++++++++++
drivers/dma/omap-dma.c | 29 +++-
drivers/dma/ti-dma-crossbar.c | 185 +++++++++++++++++++++
include/linux/dmaengine.h | 17 ++
include/linux/of_dma.h | 21 +++
11 files changed, 457 insertions(+), 30 deletions(-)
create mode 100644 Documentation/devicetree/bindings/dma/ti-dma-crossbar.txt
create mode 100644 drivers/dma/ti-dma-crossbar.c
--
2.3.5
DMA routers are transparent devices used to mux DMA requests from
peripherals to DMA controllers. They are used when the SoC integrates more
devices with DMA requests then their controller can handle.
DRA7x is one example of such SoC, where the sDMA can hanlde 128 DMA request
lines, but in SoC level it has 205 DMA requests.
The of_dma_router will be registered as of_dma_controller with special
xlate function and additional parameters. The driver for the router is
responsible to craft the dma_spec (in the of_dma_route_allocate callback)
which can be used to requests a DMA channel from the real DMA controller.
This way the router can be transparent for the system while remaining generic
enough to be used in different environments.
Signed-off-by: Peter Ujfalusi <[email protected]>
---
Documentation/devicetree/bindings/dma/dma.txt | 28 +++++++++
drivers/dma/dmaengine.c | 7 +++
drivers/dma/of-dma.c | 86 +++++++++++++++++++++++++++
include/linux/dmaengine.h | 17 ++++++
include/linux/of_dma.h | 21 +++++++
5 files changed, 159 insertions(+)
diff --git a/Documentation/devicetree/bindings/dma/dma.txt b/Documentation/devicetree/bindings/dma/dma.txt
index 82104271e754..6312fb00ce8d 100644
--- a/Documentation/devicetree/bindings/dma/dma.txt
+++ b/Documentation/devicetree/bindings/dma/dma.txt
@@ -31,6 +31,34 @@ Example:
dma-requests = <127>;
};
+* DMA router
+
+DMA routers are transparent IP blocks used to route DMA request lines from
+devices to the DMA controller. Some SoCs (like TI DRA7x) have more peripherals
+integrated with DMA requests than what the DMA controller can handle directly.
+
+Required property:
+- dma-masters: phandle of the DMA controller or list of phandles for
+ the DMA controllers the router can direct the signal to.
+- #dma-cells: Must be at least 1. Used to provide DMA router specific
+ information. See DMA client binding below for more
+ details.
+
+Optional properties:
+- dma-requests: Number of incoming request lines the router can handle.
+- In the node pointed by the dma-masters:
+ - dma-requests: The router driver might need to look for this in order
+ to configure the routing.
+
+Example:
+ sdma_xbar: dma-router@4a002b78 {
+ compatible = "ti,dra7-dma-crossbar";
+ reg = <0x4a002b78 0xfc>;
+ #dma-cells = <1>;
+ dma-requests = <205>;
+ ti,dma-safe-map = <0>;
+ dma-masters = <&sdma>;
+ };
* DMA client
diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index 24967c89f5d4..c7aa232b4a59 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -267,6 +267,13 @@ static void dma_chan_put(struct dma_chan *chan)
/* This channel is not in use anymore, free it */
if (!chan->client_count && chan->device->device_free_chan_resources)
chan->device->device_free_chan_resources(chan);
+
+ /* If the channel is used via a DMA request router, free the mapping */
+ if (chan->router && chan->router->route_free) {
+ chan->router->route_free(chan->router->dev, chan->route_data);
+ chan->router = NULL;
+ chan->route_data = NULL;
+ }
}
enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie)
diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
index cbd4a8aff120..98b3b61624f0 100644
--- a/drivers/dma/of-dma.c
+++ b/drivers/dma/of-dma.c
@@ -45,6 +45,47 @@ static struct of_dma *of_dma_find_controller(struct of_phandle_args *dma_spec)
}
/**
+ * of_dma_router_xlate - translation function for router devices
+ * @dma_spec: pointer to DMA specifier as found in the device tree
+ * @of_dma: pointer to DMA controller data (router information)
+ *
+ * The function creates new dma_spec to be passed to the router driver's
+ * of_dma_route_allocate() function to prepare a dma_spec which will be used
+ * to request channel from the real DMA controller.
+ */
+static struct dma_chan *of_dma_router_xlate(struct of_phandle_args *dma_spec,
+ struct of_dma *ofdma)
+{
+ struct dma_chan *chan;
+ struct of_dma *ofdma_target;
+ struct of_phandle_args dma_spec_target;
+ void *route_data;
+
+ /* translate the request for the real DMA controller */
+ memcpy(&dma_spec_target, dma_spec, sizeof(dma_spec_target));
+ route_data = ofdma->of_dma_route_allocate(&dma_spec_target, ofdma);
+
+ ofdma_target = of_dma_find_controller(&dma_spec_target);
+ if (!ofdma_target)
+ return NULL;
+
+ chan = ofdma_target->of_dma_xlate(&dma_spec_target, ofdma_target);
+ if (chan) {
+ chan->router = ofdma->dma_router;
+ chan->route_data = route_data;
+ } else {
+ ofdma->dma_router->route_free(ofdma->dma_router->dev, route_data);
+ }
+
+ /*
+ * Need to put the node back since the ofdma->of_dma_route_allocate
+ * has taken it for generating the new, translated dma_spec
+ */
+ of_node_put(dma_spec_target.np);
+ return chan;
+}
+
+/**
* of_dma_controller_register - Register a DMA controller to DT DMA helpers
* @np: device node of DMA controller
* @of_dma_xlate: translation function which converts a phandle
@@ -110,6 +151,51 @@ void of_dma_controller_free(struct device_node *np)
EXPORT_SYMBOL_GPL(of_dma_controller_free);
/**
+ * of_dma_router_register - Register a DMA router to DT DMA helpers as a
+ * controller
+ * @np: device node of DMA router
+ * @of_dma_route_allocate: setup function for the router which need to
+ * modify the dma_spec for the DMA controller to
+ * use and to set up the requested route.
+ * @dma_router: pointer to dma_router structure to be used when
+ * the route need to be free up.
+ *
+ * Returns 0 on success or appropriate errno value on error.
+ *
+ * Allocated memory should be freed with appropriate of_dma_controller_free()
+ * call.
+ */
+int of_dma_router_register(struct device_node *np,
+ void *(*of_dma_route_allocate)
+ (struct of_phandle_args *, struct of_dma *),
+ struct dma_router *dma_router)
+{
+ struct of_dma *ofdma;
+
+ if (!np || !of_dma_route_allocate || !dma_router) {
+ pr_err("%s: not enough information provided\n", __func__);
+ return -EINVAL;
+ }
+
+ ofdma = kzalloc(sizeof(*ofdma), GFP_KERNEL);
+ if (!ofdma)
+ return -ENOMEM;
+
+ ofdma->of_node = np;
+ ofdma->of_dma_xlate = of_dma_router_xlate;
+ ofdma->of_dma_route_allocate = of_dma_route_allocate;
+ ofdma->dma_router = dma_router;
+
+ /* Now queue of_dma controller structure in list */
+ mutex_lock(&of_dma_lock);
+ list_add_tail(&ofdma->of_dma_controllers, &of_dma_list);
+ mutex_unlock(&of_dma_lock);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(of_dma_router_register);
+
+/**
* of_dma_match_channel - Check if a DMA specifier matches name
* @np: device node to look for DMA channels
* @name: channel name to be matched
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index ad419757241f..abf63ceabef9 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -222,6 +222,16 @@ struct dma_chan_percpu {
};
/**
+ * struct dma_router - DMA router structure
+ * @dev: pointer to the DMA router device
+ * @route_free: function to be called when the route can be disconnected
+ */
+struct dma_router {
+ struct device *dev;
+ void (*route_free)(struct device *dev, void *route_data);
+};
+
+/**
* struct dma_chan - devices supply DMA channels, clients use them
* @device: ptr to the dma device who supplies this channel, always !%NULL
* @cookie: last cookie value returned to client
@@ -232,6 +242,8 @@ struct dma_chan_percpu {
* @local: per-cpu pointer to a struct dma_chan_percpu
* @client_count: how many clients are using this channel
* @table_count: number of appearances in the mem-to-mem allocation table
+ * @router: pointer to the DMA router structure
+ * @route_data: channel specific data for the router
* @private: private data for certain client-channel associations
*/
struct dma_chan {
@@ -247,6 +259,11 @@ struct dma_chan {
struct dma_chan_percpu __percpu *local;
int client_count;
int table_count;
+
+ /* DMA router */
+ struct dma_router *router;
+ void *route_data;
+
void *private;
};
diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h
index 56bc026c143f..734e449f87c1 100644
--- a/include/linux/of_dma.h
+++ b/include/linux/of_dma.h
@@ -23,6 +23,9 @@ struct of_dma {
struct device_node *of_node;
struct dma_chan *(*of_dma_xlate)
(struct of_phandle_args *, struct of_dma *);
+ void *(*of_dma_route_allocate)
+ (struct of_phandle_args *, struct of_dma *);
+ struct dma_router *dma_router;
void *of_dma_data;
};
@@ -37,12 +40,20 @@ extern int of_dma_controller_register(struct device_node *np,
(struct of_phandle_args *, struct of_dma *),
void *data);
extern void of_dma_controller_free(struct device_node *np);
+
+extern int of_dma_router_register(struct device_node *np,
+ void *(*of_dma_route_allocate)
+ (struct of_phandle_args *, struct of_dma *),
+ struct dma_router *dma_router);
+#define of_dma_router_free of_dma_controller_free
+
extern struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
const char *name);
extern struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
struct of_dma *ofdma);
extern struct dma_chan *of_dma_xlate_by_chan_id(struct of_phandle_args *dma_spec,
struct of_dma *ofdma);
+
#else
static inline int of_dma_controller_register(struct device_node *np,
struct dma_chan *(*of_dma_xlate)
@@ -56,6 +67,16 @@ static inline void of_dma_controller_free(struct device_node *np)
{
}
+static inline int of_dma_router_register(struct device_node *np,
+ void *(*of_dma_route_allocate)
+ (struct of_phandle_args *, struct of_dma *),
+ struct dma_router *dma_router)
+{
+ return -ENODEV;
+}
+
+#define of_dma_router_free of_dma_controller_free
+
static inline struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
const char *name)
{
--
2.3.5
The DRA7x has more peripherals with DMA requests than the sDMA can handle:
205 vs 127. All DMA requests are routed through the DMA crossbar, which can
be configured to route selected incoming DMA requests to specific request
line of the DMA controller.
Signed-off-by: Peter Ujfalusi <[email protected]>
---
.../devicetree/bindings/dma/ti-dma-crossbar.txt | 52 ++++++++++++++++++++++
1 file changed, 52 insertions(+)
create mode 100644 Documentation/devicetree/bindings/dma/ti-dma-crossbar.txt
diff --git a/Documentation/devicetree/bindings/dma/ti-dma-crossbar.txt b/Documentation/devicetree/bindings/dma/ti-dma-crossbar.txt
new file mode 100644
index 000000000000..63a48928f3a8
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/ti-dma-crossbar.txt
@@ -0,0 +1,52 @@
+Texas Instruments DMA Crossbar (DMA request router)
+
+Required properties:
+- compatible: "ti,dra7-dma-crossbar" for DRA7xx DMA crossbar
+- reg: Memory map for accessing module
+- #dma-cells: Should be set to <1>.
+ Clients should use the crossbar request number (input)
+- dma-requests: Number of DMA requests the crossbar can receive
+- dma-masters: phandle pointing to the DMA controller
+
+The DMA controller node need to have the following poroperties:
+- dma-requests: Number of DMA requests the controller can handle
+
+Optional properties:
+- ti,dma-safe-map: Safe routing value for unused request lines
+
+Example:
+
+/* DMA controller */
+sdma: dma-controller@4a056000 {
+ compatible = "ti,omap4430-sdma";
+ reg = <0x4a056000 0x1000>;
+ interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
+ #dma-cells = <1>;
+ dma-channels = <32>;
+ dma-requests = <127>;
+};
+
+/* DMA crossbar */
+sdma_xbar: dma-router@4a002b78 {
+ compatible = "ti,dra7-dma-crossbar";
+ reg = <0x4a002b78 0xfc>;
+ #dma-cells = <1>;
+ dma-requests = <205>;
+ ti,dma-safe-map = <0>;
+ dma-masters = <&sdma>;
+};
+
+/* DMA client */
+uart1: serial@4806a000 {
+ compatible = "ti,omap4-uart";
+ reg = <0x4806a000 0x100>;
+ interrupts-extended = <&gic GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>;
+ ti,hwmods = "uart1";
+ clock-frequency = <48000000>;
+ status = "disabled";
+ dmas = <&sdma_xbar 49>, <&sdma_xbar 50>;
+ dma-names = "tx", "rx";
+};
--
2.3.5
The DRA7x has more peripherals with DMA requests than the sDMA can handle:
205 vs 127. All DMA requests are routed through the DMA crossbar, which can
be configured to route selected incoming DMA requests to specific sDMA
request.
Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/dma/Kconfig | 4 +
drivers/dma/Makefile | 1 +
drivers/dma/ti-dma-crossbar.c | 185 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 190 insertions(+)
create mode 100644 drivers/dma/ti-dma-crossbar.c
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 91eced044321..33a7401597be 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -234,6 +234,9 @@ config TI_EDMA
Enable support for the TI EDMA controller. This DMA
engine is found on TI DaVinci and AM33xx parts.
+config TI_DMA_CROSSBAR
+ bool
+
config ARCH_HAS_ASYNC_TX_FIND_CHANNEL
bool
@@ -319,6 +322,7 @@ config DMA_OMAP
depends on ARCH_OMAP
select DMA_ENGINE
select DMA_VIRTUAL_CHANNELS
+ select TI_DMA_CROSSBAR if SOC_DRA7XX
config DMA_BCM2835
tristate "BCM2835 DMA engine support"
diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
index 7e8301cb489d..19ac70b8af0a 100644
--- a/drivers/dma/Makefile
+++ b/drivers/dma/Makefile
@@ -38,6 +38,7 @@ obj-$(CONFIG_EP93XX_DMA) += ep93xx_dma.o
obj-$(CONFIG_DMA_SA11X0) += sa11x0-dma.o
obj-$(CONFIG_MMP_TDMA) += mmp_tdma.o
obj-$(CONFIG_DMA_OMAP) += omap-dma.o
+obj-$(CONFIG_TI_DMA_CROSSBAR) += ti-dma-crossbar.o
obj-$(CONFIG_DMA_BCM2835) += bcm2835-dma.o
obj-$(CONFIG_MMP_PDMA) += mmp_pdma.o
obj-$(CONFIG_DMA_JZ4740) += dma-jz4740.o
diff --git a/drivers/dma/ti-dma-crossbar.c b/drivers/dma/ti-dma-crossbar.c
new file mode 100644
index 000000000000..5cb1eb7e86d2
--- /dev/null
+++ b/drivers/dma/ti-dma-crossbar.c
@@ -0,0 +1,185 @@
+/*
+ * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com
+ * Author: Peter Ujfalusi <[email protected]>
+ *
+ * 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/slab.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/list.h>
+#include <linux/io.h>
+#include <linux/idr.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/of_dma.h>
+
+#define TI_XBAR_OUTPUTS 127
+#define TI_XBAR_INPUTS 256
+
+static DEFINE_IDR(map_idr);
+
+struct ti_dma_xbar_data {
+ void __iomem *iomem;
+
+ struct dma_router dmarouter;
+
+ uint safe_val; /* Value to rest the crossbar lines */
+ uint xbar_requests; /* number of DMA requests connected to XBAR */
+ uint dma_requests; /* number of DMA requests forwarded to DMA */
+};
+
+struct ti_dma_xbar_map {
+ int xbar_in;
+ int xbar_out;
+};
+
+static inline void ti_dma_xbar_write(void __iomem *iomem, int xbar, int val)
+{
+ writew_relaxed(val, iomem + (xbar * 2));
+}
+
+static void ti_dma_xbar_free(struct device *dev, void *route_data)
+{
+ struct ti_dma_xbar_data *xbar = dev_get_drvdata(dev);
+ struct ti_dma_xbar_map *map = route_data;
+
+ dev_dbg(dev, "Unmapping XBAR%d (was routed to %d)\n",
+ map->xbar_in, map->xbar_out);
+
+ ti_dma_xbar_write(xbar->iomem, map->xbar_out, xbar->safe_val);
+ idr_remove(&map_idr, map->xbar_out);
+ kfree(map);
+}
+
+static void *ti_dma_xbar_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 ti_dma_xbar_data *xbar = platform_get_drvdata(pdev);
+ struct ti_dma_xbar_map *map;
+
+ if (dma_spec->args[0] >= xbar->xbar_requests) {
+ dev_err(&pdev->dev, "Invalid XBAR request number: %d\n",
+ dma_spec->args[0]);
+ return NULL;
+ }
+
+ 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 NULL;
+ }
+
+ map = kzalloc(sizeof(*map), GFP_KERNEL);
+ if (!map)
+ return NULL;
+
+ map->xbar_out = idr_alloc(&map_idr, NULL, 0, xbar->dma_requests,
+ GFP_KERNEL);
+ map->xbar_in = dma_spec->args[0];
+
+ /* The DMA request is 1 based in sDMA */
+ dma_spec->args[0] = map->xbar_out + 1;
+
+ dev_dbg(&pdev->dev, "Mapping XBAR%d to DMA%d\n",
+ map->xbar_in, map->xbar_out);
+
+ ti_dma_xbar_write(xbar->iomem, map->xbar_out, map->xbar_in);
+
+ return map;
+}
+
+static int ti_dma_xbar_probe(struct platform_device *pdev)
+{
+ struct device_node *node = pdev->dev.of_node;
+ struct device_node *dma_node;
+ struct ti_dma_xbar_data *xbar;
+ struct resource *res;
+ void __iomem *iomem;
+ int i, ret;
+
+ if (!node)
+ return -ENODEV;
+
+ 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;
+ }
+
+ xbar = devm_kzalloc(&pdev->dev, sizeof(*xbar), GFP_KERNEL);
+ if (!xbar)
+ return -ENOMEM;
+
+ if (of_property_read_u32(dma_node, "dma-requests",
+ &xbar->dma_requests)) {
+ dev_info(&pdev->dev,
+ "Missing XBAR output information, using %u.\n",
+ TI_XBAR_OUTPUTS);
+ xbar->dma_requests = TI_XBAR_OUTPUTS;
+ }
+ of_node_put(dma_node);
+
+ if (of_property_read_u32(node, "dma-requests", &xbar->xbar_requests)) {
+ dev_info(&pdev->dev,
+ "Missing XBAR input information, using %u.\n",
+ TI_XBAR_INPUTS);
+ xbar->xbar_requests = TI_XBAR_INPUTS;
+ }
+
+ if (of_property_read_u32(node, "ti,dma-safe-map", &xbar->safe_val))
+ xbar->safe_val = 0;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -ENODEV;
+
+ if (!devm_request_mem_region(&pdev->dev, res->start, resource_size(res),
+ dev_name(&pdev->dev)))
+ return -ENODEV;
+
+ iomem = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+ if (!iomem)
+ return -ENOMEM;
+
+ xbar->iomem = iomem;
+
+ xbar->dmarouter.dev = &pdev->dev;
+ xbar->dmarouter.route_free = ti_dma_xbar_free;
+
+ platform_set_drvdata(pdev, xbar);
+
+ ret = of_dma_router_register(node, ti_dma_xbar_route_allocate,
+ &xbar->dmarouter);
+ if (ret)
+ return -ENODEV;
+
+ /* Reset the crossbar */
+ for (i = 0; i < xbar->dma_requests; i++)
+ ti_dma_xbar_write(xbar->iomem, i, xbar->safe_val);
+
+ return 0;
+}
+
+static const struct of_device_id ti_dma_xbar_match[] = {
+ { .compatible = "ti,dra7-dma-crossbar" },
+ {},
+};
+
+static struct platform_driver ti_dma_xbar_driver = {
+ .driver = {
+ .name = "ti-dma-crossbar",
+ .of_match_table = of_match_ptr(ti_dma_xbar_match),
+ },
+ .probe = ti_dma_xbar_probe,
+};
+
+int omap_dmaxbar_init(void)
+{
+ return platform_driver_register(&ti_dma_xbar_driver);
+}
+arch_initcall(omap_dmaxbar_init);
--
2.3.5
Instead of magic numbers in the code, use define for number of logical DMA
channels and DMA requests.
Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/dma/omap-dma.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index 167dbaf65742..4065963fbffb 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -22,6 +22,9 @@
#include "virt-dma.h"
+#define OMAP_SDMA_REQUESTS 127
+#define OMAP_SDMA_CHANNELS 32
+
struct omap_dmadev {
struct dma_device ddev;
spinlock_t lock;
@@ -33,7 +36,7 @@ struct omap_dmadev {
bool legacy;
spinlock_t irq_lock;
uint32_t irq_enable_mask;
- struct omap_chan *lch_map[32];
+ struct omap_chan *lch_map[OMAP_SDMA_CHANNELS];
};
struct omap_chan {
@@ -1116,7 +1119,7 @@ static int omap_dma_probe(struct platform_device *pdev)
tasklet_init(&od->task, omap_dma_sched, (unsigned long)od);
- for (i = 0; i < 127; i++) {
+ for (i = 0; i < OMAP_SDMA_REQUESTS; i++) {
rc = omap_dma_chan_init(od, i);
if (rc) {
omap_dma_free(od);
--
2.3.5
Use the dma-requests property from DT to get the number of DMA requests.
In case of legacy boot or failure to find the property, use the default
127 as number of requests.
Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/dma/omap-dma.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index 4065963fbffb..8c41cdb55a4b 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -34,6 +34,7 @@ struct omap_dmadev {
const struct omap_dma_reg *reg_map;
struct omap_system_dma_plat_info *plat;
bool legacy;
+ unsigned dma_requests;
spinlock_t irq_lock;
uint32_t irq_enable_mask;
struct omap_chan *lch_map[OMAP_SDMA_CHANNELS];
@@ -1119,7 +1120,16 @@ static int omap_dma_probe(struct platform_device *pdev)
tasklet_init(&od->task, omap_dma_sched, (unsigned long)od);
- for (i = 0; i < OMAP_SDMA_REQUESTS; i++) {
+ od->dma_requests = OMAP_SDMA_REQUESTS;
+ if (pdev->dev.of_node && of_property_read_u32(pdev->dev.of_node,
+ "dma-requests",
+ &od->dma_requests)) {
+ dev_info(&pdev->dev,
+ "Missing dma-requests property, using %u.\n",
+ OMAP_SDMA_REQUESTS);
+ }
+
+ for (i = 0; i < od->dma_requests; i++) {
rc = omap_dma_chan_init(od, i);
if (rc) {
omap_dma_free(od);
--
2.3.5
Do not direct map the virtual channels to sDMA request number. When the
sDMA is behind of a crossbar this direct mapping can cause situations when
certain channel can not be requested since the crossbar request number
will no longer match with the sDMA request line.
The direct mapping for virtual channels with HW request lines will make it
harder to implement MEM_TO_MEM mode for the driver.
Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/dma/omap-dma.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index 8c41cdb55a4b..5ece72c30216 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -593,6 +593,7 @@ static void omap_dma_free_chan_resources(struct dma_chan *chan)
omap_free_dma(c->dma_ch);
dev_dbg(od->ddev.dev, "freeing channel for %u\n", c->dma_sig);
+ c->dma_sig = 0;
}
static size_t omap_dma_sg_size(struct omap_sg *sg)
@@ -1041,7 +1042,7 @@ static int omap_dma_resume(struct dma_chan *chan)
return 0;
}
-static int omap_dma_chan_init(struct omap_dmadev *od, int dma_sig)
+static int omap_dma_chan_init(struct omap_dmadev *od)
{
struct omap_chan *c;
@@ -1050,7 +1051,6 @@ static int omap_dma_chan_init(struct omap_dmadev *od, int dma_sig)
return -ENOMEM;
c->reg_map = od->reg_map;
- c->dma_sig = dma_sig;
c->vc.desc_free = omap_dma_desc_free;
vchan_init(&c->vc, &od->ddev);
INIT_LIST_HEAD(&c->node);
@@ -1130,7 +1130,7 @@ static int omap_dma_probe(struct platform_device *pdev)
}
for (i = 0; i < od->dma_requests; i++) {
- rc = omap_dma_chan_init(od, i);
+ rc = omap_dma_chan_init(od);
if (rc) {
omap_dma_free(od);
return rc;
@@ -1221,10 +1221,14 @@ static struct platform_driver omap_dma_driver = {
bool omap_dma_filter_fn(struct dma_chan *chan, void *param)
{
if (chan->device->dev->driver == &omap_dma_driver.driver) {
+ struct omap_dmadev *od = to_omap_dma_dev(chan->device);
struct omap_chan *c = to_omap_dma_chan(chan);
unsigned req = *(unsigned *)param;
- return req == c->dma_sig;
+ if (req <= od->dma_requests) {
+ c->dma_sig = req;
+ return true;
+ }
}
return false;
}
--
2.3.5
Since the mapping between the hardware request lines and channels has been
removed it no longer make sense to have too many channels.
Set the number of channels to match with the number of logical channels
supported by sDMA.
Signed-off-by: Peter Ujfalusi <[email protected]>
---
drivers/dma/omap-dma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index 5ece72c30216..66513fc24e83 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -1129,7 +1129,7 @@ static int omap_dma_probe(struct platform_device *pdev)
OMAP_SDMA_REQUESTS);
}
- for (i = 0; i < od->dma_requests; i++) {
+ for (i = 0; i < OMAP_SDMA_CHANNELS; i++) {
rc = omap_dma_chan_init(od);
if (rc) {
omap_dma_free(od);
--
2.3.5
The sDMA requests are routed through the DMA crossbar and without the
crossbar only peripherals using DMA request 0-127 can be used.
Signed-off-by: Peter Ujfalusi <[email protected]>
---
arch/arm/boot/dts/dra7.dtsi | 57 ++++++++++++++++++++++++++-------------------
1 file changed, 33 insertions(+), 24 deletions(-)
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index 5332b57b4950..a2f4aa01984f 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -303,6 +303,15 @@
dma-requests = <127>;
};
+ sdma_xbar: dma-router@4a002b78 {
+ compatible = "ti,dra7-dma-crossbar";
+ reg = <0x4a002b78 0xfc>;
+ #dma-cells = <1>;
+ dma-requests = <205>;
+ ti,dma-safe-map = <0>;
+ dma-masters = <&sdma>;
+ };
+
gpio1: gpio@4ae10000 {
compatible = "ti,omap4-gpio";
reg = <0x4ae10000 0x200>;
@@ -398,7 +407,7 @@
ti,hwmods = "uart1";
clock-frequency = <48000000>;
status = "disabled";
- dmas = <&sdma 49>, <&sdma 50>;
+ dmas = <&sdma_xbar 49>, <&sdma_xbar 50>;
dma-names = "tx", "rx";
};
@@ -409,7 +418,7 @@
ti,hwmods = "uart2";
clock-frequency = <48000000>;
status = "disabled";
- dmas = <&sdma 51>, <&sdma 52>;
+ dmas = <&sdma_xbar 51>, <&sdma_xbar 52>;
dma-names = "tx", "rx";
};
@@ -420,7 +429,7 @@
ti,hwmods = "uart3";
clock-frequency = <48000000>;
status = "disabled";
- dmas = <&sdma 53>, <&sdma 54>;
+ dmas = <&sdma_xbar 53>, <&sdma_xbar 54>;
dma-names = "tx", "rx";
};
@@ -431,7 +440,7 @@
ti,hwmods = "uart4";
clock-frequency = <48000000>;
status = "disabled";
- dmas = <&sdma 55>, <&sdma 56>;
+ dmas = <&sdma_xbar 55>, <&sdma_xbar 56>;
dma-names = "tx", "rx";
};
@@ -442,7 +451,7 @@
ti,hwmods = "uart5";
clock-frequency = <48000000>;
status = "disabled";
- dmas = <&sdma 63>, <&sdma 64>;
+ dmas = <&sdma_xbar 63>, <&sdma_xbar 64>;
dma-names = "tx", "rx";
};
@@ -453,7 +462,7 @@
ti,hwmods = "uart6";
clock-frequency = <48000000>;
status = "disabled";
- dmas = <&sdma 79>, <&sdma 80>;
+ dmas = <&sdma_xbar 79>, <&sdma_xbar 80>;
dma-names = "tx", "rx";
};
@@ -862,7 +871,7 @@
ti,hwmods = "mmc1";
ti,dual-volt;
ti,needs-special-reset;
- dmas = <&sdma 61>, <&sdma 62>;
+ dmas = <&sdma_xbar 61>, <&sdma_xbar 62>;
dma-names = "tx", "rx";
status = "disabled";
pbias-supply = <&pbias_mmc_reg>;
@@ -874,7 +883,7 @@
interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_HIGH>;
ti,hwmods = "mmc2";
ti,needs-special-reset;
- dmas = <&sdma 47>, <&sdma 48>;
+ dmas = <&sdma_xbar 47>, <&sdma_xbar 48>;
dma-names = "tx", "rx";
status = "disabled";
};
@@ -885,7 +894,7 @@
interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
ti,hwmods = "mmc3";
ti,needs-special-reset;
- dmas = <&sdma 77>, <&sdma 78>;
+ dmas = <&sdma_xbar 77>, <&sdma_xbar 78>;
dma-names = "tx", "rx";
status = "disabled";
};
@@ -896,7 +905,7 @@
interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
ti,hwmods = "mmc4";
ti,needs-special-reset;
- dmas = <&sdma 57>, <&sdma 58>;
+ dmas = <&sdma_xbar 57>, <&sdma_xbar 58>;
dma-names = "tx", "rx";
status = "disabled";
};
@@ -1041,14 +1050,14 @@
#size-cells = <0>;
ti,hwmods = "mcspi1";
ti,spi-num-cs = <4>;
- dmas = <&sdma 35>,
- <&sdma 36>,
- <&sdma 37>,
- <&sdma 38>,
- <&sdma 39>,
- <&sdma 40>,
- <&sdma 41>,
- <&sdma 42>;
+ dmas = <&sdma_xbar 35>,
+ <&sdma_xbar 36>,
+ <&sdma_xbar 37>,
+ <&sdma_xbar 38>,
+ <&sdma_xbar 39>,
+ <&sdma_xbar 40>,
+ <&sdma_xbar 41>,
+ <&sdma_xbar 42>;
dma-names = "tx0", "rx0", "tx1", "rx1",
"tx2", "rx2", "tx3", "rx3";
status = "disabled";
@@ -1062,10 +1071,10 @@
#size-cells = <0>;
ti,hwmods = "mcspi2";
ti,spi-num-cs = <2>;
- dmas = <&sdma 43>,
- <&sdma 44>,
- <&sdma 45>,
- <&sdma 46>;
+ dmas = <&sdma_xbar 43>,
+ <&sdma_xbar 44>,
+ <&sdma_xbar 45>,
+ <&sdma_xbar 46>;
dma-names = "tx0", "rx0", "tx1", "rx1";
status = "disabled";
};
@@ -1078,7 +1087,7 @@
#size-cells = <0>;
ti,hwmods = "mcspi3";
ti,spi-num-cs = <2>;
- dmas = <&sdma 15>, <&sdma 16>;
+ dmas = <&sdma_xbar 15>, <&sdma_xbar 16>;
dma-names = "tx0", "rx0";
status = "disabled";
};
@@ -1091,7 +1100,7 @@
#size-cells = <0>;
ti,hwmods = "mcspi4";
ti,spi-num-cs = <1>;
- dmas = <&sdma 70>, <&sdma 71>;
+ dmas = <&sdma_xbar 70>, <&sdma_xbar 71>;
dma-names = "tx0", "rx0";
status = "disabled";
};
--
2.3.5
Hi,
On Wed, Apr 08, 2015 at 04:14:47PM +0300, Peter Ujfalusi wrote:
> The DRA7x has more peripherals with DMA requests than the sDMA can handle:
> 205 vs 127. All DMA requests are routed through the DMA crossbar, which can
> be configured to route selected incoming DMA requests to specific sDMA
> request.
>
> Signed-off-by: Peter Ujfalusi <[email protected]>
> ---
> drivers/dma/Kconfig | 4 +
> drivers/dma/Makefile | 1 +
> drivers/dma/ti-dma-crossbar.c | 185 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 190 insertions(+)
> create mode 100644 drivers/dma/ti-dma-crossbar.c
>
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index 91eced044321..33a7401597be 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -234,6 +234,9 @@ config TI_EDMA
> Enable support for the TI EDMA controller. This DMA
> engine is found on TI DaVinci and AM33xx parts.
>
> +config TI_DMA_CROSSBAR
> + bool
> +
> config ARCH_HAS_ASYNC_TX_FIND_CHANNEL
> bool
>
> @@ -319,6 +322,7 @@ config DMA_OMAP
> depends on ARCH_OMAP
> select DMA_ENGINE
> select DMA_VIRTUAL_CHANNELS
> + select TI_DMA_CROSSBAR if SOC_DRA7XX
>
> config DMA_BCM2835
> tristate "BCM2835 DMA engine support"
> diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> index 7e8301cb489d..19ac70b8af0a 100644
> --- a/drivers/dma/Makefile
> +++ b/drivers/dma/Makefile
> @@ -38,6 +38,7 @@ obj-$(CONFIG_EP93XX_DMA) += ep93xx_dma.o
> obj-$(CONFIG_DMA_SA11X0) += sa11x0-dma.o
> obj-$(CONFIG_MMP_TDMA) += mmp_tdma.o
> obj-$(CONFIG_DMA_OMAP) += omap-dma.o
> +obj-$(CONFIG_TI_DMA_CROSSBAR) += ti-dma-crossbar.o
> obj-$(CONFIG_DMA_BCM2835) += bcm2835-dma.o
> obj-$(CONFIG_MMP_PDMA) += mmp_pdma.o
> obj-$(CONFIG_DMA_JZ4740) += dma-jz4740.o
> diff --git a/drivers/dma/ti-dma-crossbar.c b/drivers/dma/ti-dma-crossbar.c
> new file mode 100644
> index 000000000000..5cb1eb7e86d2
> --- /dev/null
> +++ b/drivers/dma/ti-dma-crossbar.c
> @@ -0,0 +1,185 @@
> +/*
> + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com
> + * Author: Peter Ujfalusi <[email protected]>
> + *
> + * 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/slab.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/list.h>
> +#include <linux/io.h>
> +#include <linux/idr.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/of_dma.h>
> +
> +#define TI_XBAR_OUTPUTS 127
> +#define TI_XBAR_INPUTS 256
> +
> +static DEFINE_IDR(map_idr);
> +
> +struct ti_dma_xbar_data {
> + void __iomem *iomem;
> +
> + struct dma_router dmarouter;
> +
> + uint safe_val; /* Value to rest the crossbar lines */
> + uint xbar_requests; /* number of DMA requests connected to XBAR */
> + uint dma_requests; /* number of DMA requests forwarded to DMA */
> +};
> +
> +struct ti_dma_xbar_map {
> + int xbar_in;
> + int xbar_out;
> +};
> +
> +static inline void ti_dma_xbar_write(void __iomem *iomem, int xbar, int val)
> +{
> + writew_relaxed(val, iomem + (xbar * 2));
Silently casting val (an integer) to a u16 isn't really nice I
guess. At least you could be upfront about it in the prototype.
> +}
> +
> +static void ti_dma_xbar_free(struct device *dev, void *route_data)
> +{
> + struct ti_dma_xbar_data *xbar = dev_get_drvdata(dev);
> + struct ti_dma_xbar_map *map = route_data;
> +
> + dev_dbg(dev, "Unmapping XBAR%d (was routed to %d)\n",
> + map->xbar_in, map->xbar_out);
> +
> + ti_dma_xbar_write(xbar->iomem, map->xbar_out, xbar->safe_val);
> + idr_remove(&map_idr, map->xbar_out);
> + kfree(map);
> +}
> +
> +static void *ti_dma_xbar_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 ti_dma_xbar_data *xbar = platform_get_drvdata(pdev);
> + struct ti_dma_xbar_map *map;
> +
> + if (dma_spec->args[0] >= xbar->xbar_requests) {
> + dev_err(&pdev->dev, "Invalid XBAR request number: %d\n",
> + dma_spec->args[0]);
> + return NULL;
> + }
> +
> + 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 NULL;
> + }
> +
> + map = kzalloc(sizeof(*map), GFP_KERNEL);
> + if (!map)
> + return NULL;
You're leaking dma_spec->np.
> +
> + map->xbar_out = idr_alloc(&map_idr, NULL, 0, xbar->dma_requests,
> + GFP_KERNEL);
> + map->xbar_in = dma_spec->args[0];
> +
> + /* The DMA request is 1 based in sDMA */
> + dma_spec->args[0] = map->xbar_out + 1;
> +
> + dev_dbg(&pdev->dev, "Mapping XBAR%d to DMA%d\n",
> + map->xbar_in, map->xbar_out);
> +
> + ti_dma_xbar_write(xbar->iomem, map->xbar_out, map->xbar_in);
> +
> + return map;
> +}
> +
> +static int ti_dma_xbar_probe(struct platform_device *pdev)
> +{
> + struct device_node *node = pdev->dev.of_node;
> + struct device_node *dma_node;
> + struct ti_dma_xbar_data *xbar;
> + struct resource *res;
> + void __iomem *iomem;
> + int i, ret;
> +
> + if (!node)
> + return -ENODEV;
> +
> + 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;
> + }
> +
> + xbar = devm_kzalloc(&pdev->dev, sizeof(*xbar), GFP_KERNEL);
> + if (!xbar)
> + return -ENOMEM;
And here too.
> + if (of_property_read_u32(dma_node, "dma-requests",
> + &xbar->dma_requests)) {
> + dev_info(&pdev->dev,
> + "Missing XBAR output information, using %u.\n",
> + TI_XBAR_OUTPUTS);
> + xbar->dma_requests = TI_XBAR_OUTPUTS;
> + }
> + of_node_put(dma_node);
> +
> + if (of_property_read_u32(node, "dma-requests", &xbar->xbar_requests)) {
> + dev_info(&pdev->dev,
> + "Missing XBAR input information, using %u.\n",
> + TI_XBAR_INPUTS);
> + xbar->xbar_requests = TI_XBAR_INPUTS;
> + }
> +
> + if (of_property_read_u32(node, "ti,dma-safe-map", &xbar->safe_val))
> + xbar->safe_val = 0;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -ENODEV;
> +
> + if (!devm_request_mem_region(&pdev->dev, res->start, resource_size(res),
> + dev_name(&pdev->dev)))
> + return -ENODEV;
> +
> + iomem = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> + if (!iomem)
> + return -ENOMEM;
You can use devm_ioremap_resource here.
> + xbar->iomem = iomem;
> +
> + xbar->dmarouter.dev = &pdev->dev;
> + xbar->dmarouter.route_free = ti_dma_xbar_free;
> +
> + platform_set_drvdata(pdev, xbar);
> +
> + ret = of_dma_router_register(node, ti_dma_xbar_route_allocate,
> + &xbar->dmarouter);
> + if (ret)
> + return -ENODEV;
Starting here, I'd expect that your driver can be used....
> +
> + /* Reset the crossbar */
> + for (i = 0; i < xbar->dma_requests; i++)
> + ti_dma_xbar_write(xbar->iomem, i, xbar->safe_val);
... however, the hardware doesn't seem ready. Isn't it a race?
> + return 0;
> +}
> +
> +static const struct of_device_id ti_dma_xbar_match[] = {
> + { .compatible = "ti,dra7-dma-crossbar" },
> + {},
> +};
> +
> +static struct platform_driver ti_dma_xbar_driver = {
> + .driver = {
> + .name = "ti-dma-crossbar",
> + .of_match_table = of_match_ptr(ti_dma_xbar_match),
> + },
> + .probe = ti_dma_xbar_probe,
> +};
> +
> +int omap_dmaxbar_init(void)
> +{
> + return platform_driver_register(&ti_dma_xbar_driver);
> +}
> +arch_initcall(omap_dmaxbar_init);
Thanks,
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
Hi,
On Wed, Apr 08, 2015 at 04:14:45PM +0300, Peter Ujfalusi wrote:
> DMA routers are transparent devices used to mux DMA requests from
> peripherals to DMA controllers. They are used when the SoC integrates more
> devices with DMA requests then their controller can handle.
> DRA7x is one example of such SoC, where the sDMA can hanlde 128 DMA request
> lines, but in SoC level it has 205 DMA requests.
>
> The of_dma_router will be registered as of_dma_controller with special
> xlate function and additional parameters. The driver for the router is
> responsible to craft the dma_spec (in the of_dma_route_allocate callback)
> which can be used to requests a DMA channel from the real DMA controller.
> This way the router can be transparent for the system while remaining generic
> enough to be used in different environments.
>
> Signed-off-by: Peter Ujfalusi <[email protected]>
> ---
> Documentation/devicetree/bindings/dma/dma.txt | 28 +++++++++
> drivers/dma/dmaengine.c | 7 +++
> drivers/dma/of-dma.c | 86 +++++++++++++++++++++++++++
> include/linux/dmaengine.h | 17 ++++++
> include/linux/of_dma.h | 21 +++++++
> 5 files changed, 159 insertions(+)
Can that be moved to a header / C file of its own?
There's a lot of various code already in dmaengine.h and dmaengine.c,
it would be really great to avoid adding more random stuff in there.
>
> diff --git a/Documentation/devicetree/bindings/dma/dma.txt b/Documentation/devicetree/bindings/dma/dma.txt
> index 82104271e754..6312fb00ce8d 100644
> --- a/Documentation/devicetree/bindings/dma/dma.txt
> +++ b/Documentation/devicetree/bindings/dma/dma.txt
> @@ -31,6 +31,34 @@ Example:
> dma-requests = <127>;
> };
>
> +* DMA router
> +
> +DMA routers are transparent IP blocks used to route DMA request lines from
> +devices to the DMA controller. Some SoCs (like TI DRA7x) have more peripherals
> +integrated with DMA requests than what the DMA controller can handle directly.
> +
> +Required property:
> +- dma-masters: phandle of the DMA controller or list of phandles for
> + the DMA controllers the router can direct the signal to.
> +- #dma-cells: Must be at least 1. Used to provide DMA router specific
> + information. See DMA client binding below for more
> + details.
> +
> +Optional properties:
> +- dma-requests: Number of incoming request lines the router can handle.
> +- In the node pointed by the dma-masters:
> + - dma-requests: The router driver might need to look for this in order
> + to configure the routing.
> +
> +Example:
> + sdma_xbar: dma-router@4a002b78 {
> + compatible = "ti,dra7-dma-crossbar";
> + reg = <0x4a002b78 0xfc>;
> + #dma-cells = <1>;
> + dma-requests = <205>;
> + ti,dma-safe-map = <0>;
> + dma-masters = <&sdma>;
> + };
>
> * DMA client
>
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index 24967c89f5d4..c7aa232b4a59 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -267,6 +267,13 @@ static void dma_chan_put(struct dma_chan *chan)
> /* This channel is not in use anymore, free it */
> if (!chan->client_count && chan->device->device_free_chan_resources)
> chan->device->device_free_chan_resources(chan);
> +
> + /* If the channel is used via a DMA request router, free the mapping */
> + if (chan->router && chan->router->route_free) {
> + chan->router->route_free(chan->router->dev, chan->route_data);
> + chan->router = NULL;
> + chan->route_data = NULL;
> + }
> }
>
> enum dma_status dma_sync_wait(struct dma_chan *chan, dma_cookie_t cookie)
> diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
> index cbd4a8aff120..98b3b61624f0 100644
> --- a/drivers/dma/of-dma.c
> +++ b/drivers/dma/of-dma.c
> @@ -45,6 +45,47 @@ static struct of_dma *of_dma_find_controller(struct of_phandle_args *dma_spec)
> }
>
> /**
> + * of_dma_router_xlate - translation function for router devices
> + * @dma_spec: pointer to DMA specifier as found in the device tree
> + * @of_dma: pointer to DMA controller data (router information)
> + *
> + * The function creates new dma_spec to be passed to the router driver's
> + * of_dma_route_allocate() function to prepare a dma_spec which will be used
> + * to request channel from the real DMA controller.
> + */
> +static struct dma_chan *of_dma_router_xlate(struct of_phandle_args *dma_spec,
> + struct of_dma *ofdma)
> +{
> + struct dma_chan *chan;
> + struct of_dma *ofdma_target;
> + struct of_phandle_args dma_spec_target;
> + void *route_data;
> +
> + /* translate the request for the real DMA controller */
> + memcpy(&dma_spec_target, dma_spec, sizeof(dma_spec_target));
> + route_data = ofdma->of_dma_route_allocate(&dma_spec_target, ofdma);
> +
> + ofdma_target = of_dma_find_controller(&dma_spec_target);
> + if (!ofdma_target)
> + return NULL;
> +
> + chan = ofdma_target->of_dma_xlate(&dma_spec_target, ofdma_target);
> + if (chan) {
> + chan->router = ofdma->dma_router;
> + chan->route_data = route_data;
> + } else {
> + ofdma->dma_router->route_free(ofdma->dma_router->dev, route_data);
This line is over 80 characters.
> + }
> +
> + /*
> + * Need to put the node back since the ofdma->of_dma_route_allocate
> + * has taken it for generating the new, translated dma_spec
> + */
> + of_node_put(dma_spec_target.np);
> + return chan;
> +}
> +
> +/**
> * of_dma_controller_register - Register a DMA controller to DT DMA helpers
> * @np: device node of DMA controller
> * @of_dma_xlate: translation function which converts a phandle
> @@ -110,6 +151,51 @@ void of_dma_controller_free(struct device_node *np)
> EXPORT_SYMBOL_GPL(of_dma_controller_free);
>
> /**
> + * of_dma_router_register - Register a DMA router to DT DMA helpers as a
> + * controller
> + * @np: device node of DMA router
> + * @of_dma_route_allocate: setup function for the router which need to
> + * modify the dma_spec for the DMA controller to
> + * use and to set up the requested route.
> + * @dma_router: pointer to dma_router structure to be used when
> + * the route need to be free up.
> + *
> + * Returns 0 on success or appropriate errno value on error.
> + *
> + * Allocated memory should be freed with appropriate of_dma_controller_free()
> + * call.
> + */
> +int of_dma_router_register(struct device_node *np,
> + void *(*of_dma_route_allocate)
> + (struct of_phandle_args *, struct of_dma *),
> + struct dma_router *dma_router)
> +{
> + struct of_dma *ofdma;
> +
> + if (!np || !of_dma_route_allocate || !dma_router) {
> + pr_err("%s: not enough information provided\n", __func__);
> + return -EINVAL;
> + }
> +
> + ofdma = kzalloc(sizeof(*ofdma), GFP_KERNEL);
> + if (!ofdma)
> + return -ENOMEM;
Is that expected that you allocate through kzalloc, but never have a
matching free function implemented?
> + ofdma->of_node = np;
> + ofdma->of_dma_xlate = of_dma_router_xlate;
> + ofdma->of_dma_route_allocate = of_dma_route_allocate;
> + ofdma->dma_router = dma_router;
> +
> + /* Now queue of_dma controller structure in list */
> + mutex_lock(&of_dma_lock);
> + list_add_tail(&ofdma->of_dma_controllers, &of_dma_list);
> + mutex_unlock(&of_dma_lock);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(of_dma_router_register);
> +
> +/**
> * of_dma_match_channel - Check if a DMA specifier matches name
> * @np: device node to look for DMA channels
> * @name: channel name to be matched
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index ad419757241f..abf63ceabef9 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -222,6 +222,16 @@ struct dma_chan_percpu {
> };
>
> /**
> + * struct dma_router - DMA router structure
> + * @dev: pointer to the DMA router device
> + * @route_free: function to be called when the route can be disconnected
> + */
> +struct dma_router {
> + struct device *dev;
> + void (*route_free)(struct device *dev, void *route_data);
> +};
> +
> +/**
> * struct dma_chan - devices supply DMA channels, clients use them
> * @device: ptr to the dma device who supplies this channel, always !%NULL
> * @cookie: last cookie value returned to client
> @@ -232,6 +242,8 @@ struct dma_chan_percpu {
> * @local: per-cpu pointer to a struct dma_chan_percpu
> * @client_count: how many clients are using this channel
> * @table_count: number of appearances in the mem-to-mem allocation table
> + * @router: pointer to the DMA router structure
> + * @route_data: channel specific data for the router
> * @private: private data for certain client-channel associations
> */
> struct dma_chan {
> @@ -247,6 +259,11 @@ struct dma_chan {
> struct dma_chan_percpu __percpu *local;
> int client_count;
> int table_count;
> +
> + /* DMA router */
> + struct dma_router *router;
> + void *route_data;
> +
> void *private;
> };
>
> diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h
> index 56bc026c143f..734e449f87c1 100644
> --- a/include/linux/of_dma.h
> +++ b/include/linux/of_dma.h
> @@ -23,6 +23,9 @@ struct of_dma {
> struct device_node *of_node;
> struct dma_chan *(*of_dma_xlate)
> (struct of_phandle_args *, struct of_dma *);
> + void *(*of_dma_route_allocate)
> + (struct of_phandle_args *, struct of_dma *);
> + struct dma_router *dma_router;
I don't really see why this is really tied to the device tree.
Couldn't we use the device_alloc_chan_resources to do that?
Thanks,
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
>> +static inline void ti_dma_xbar_write(void __iomem *iomem, int xbar, int val)
>> +{
>> + writew_relaxed(val, iomem + (xbar * 2));
>
> Silently casting val (an integer) to a u16 isn't really nice I
> guess. At least you could be upfront about it in the prototype.
The value in val is guarantied not to overflow the u16, but sure, I can change
this.
>> + 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 NULL;
>> + }
>> +
>> + map = kzalloc(sizeof(*map), GFP_KERNEL);
>> + if (!map)
>> + return NULL;
>
> You're leaking dma_spec->np.
If kzalloc fails we have other things to worry about, but true, I'll
restructure the code to avoid leaking the node.
>> + 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;
>> + }
>> +
>> + xbar = devm_kzalloc(&pdev->dev, sizeof(*xbar), GFP_KERNEL);
>> + if (!xbar)
>> + return -ENOMEM;
>
> And here too.
same here, I'll change the code to not leak.
>
>> + if (of_property_read_u32(dma_node, "dma-requests",
>> + &xbar->dma_requests)) {
>> + dev_info(&pdev->dev,
>> + "Missing XBAR output information, using %u.\n",
>> + TI_XBAR_OUTPUTS);
>> + xbar->dma_requests = TI_XBAR_OUTPUTS;
>> + }
>> + of_node_put(dma_node);
>> +
>> + if (of_property_read_u32(node, "dma-requests", &xbar->xbar_requests)) {
>> + dev_info(&pdev->dev,
>> + "Missing XBAR input information, using %u.\n",
>> + TI_XBAR_INPUTS);
>> + xbar->xbar_requests = TI_XBAR_INPUTS;
>> + }
>> +
>> + if (of_property_read_u32(node, "ti,dma-safe-map", &xbar->safe_val))
>> + xbar->safe_val = 0;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + if (!res)
>> + return -ENODEV;
>> +
>> + if (!devm_request_mem_region(&pdev->dev, res->start, resource_size(res),
>> + dev_name(&pdev->dev)))
>> + return -ENODEV;
>> +
>> + iomem = devm_ioremap(&pdev->dev, res->start, resource_size(res));
>> + if (!iomem)
>> + return -ENOMEM;
>
> You can use devm_ioremap_resource here.
I tend to forgot about this...
>> + xbar->iomem = iomem;
>> +
>> + xbar->dmarouter.dev = &pdev->dev;
>> + xbar->dmarouter.route_free = ti_dma_xbar_free;
>> +
>> + platform_set_drvdata(pdev, xbar);
>> +
>> + ret = of_dma_router_register(node, ti_dma_xbar_route_allocate,
>> + &xbar->dmarouter);
>> + if (ret)
>> + return -ENODEV;
>
> Starting here, I'd expect that your driver can be used....
>
>> +
>> + /* Reset the crossbar */
>> + for (i = 0; i < xbar->dma_requests; i++)
>> + ti_dma_xbar_write(xbar->iomem, i, xbar->safe_val);
>
> ... however, the hardware doesn't seem ready. Isn't it a race?
True, will change the order.
Thanks,
P?ter
On 04/08/2015 06:42 PM, Maxime Ripard wrote:
>> ---
>> Documentation/devicetree/bindings/dma/dma.txt | 28 +++++++++
>> drivers/dma/dmaengine.c | 7 +++
>> drivers/dma/of-dma.c | 86 +++++++++++++++++++++++++++
>> include/linux/dmaengine.h | 17 ++++++
>> include/linux/of_dma.h | 21 +++++++
>> 5 files changed, 159 insertions(+)
>
> Can that be moved to a header / C file of its own?
>
> There's a lot of various code already in dmaengine.h and dmaengine.c,
> it would be really great to avoid adding more random stuff in there.
This patch adds the core support for DMA signal routers. It adds fairly small
amount of generic code to the core to achieve this. I don't think it would be
better to create let's say of-dma-router.c and .h just for this and export
functions from of-dma.c to be used outside of the file.
>> + chan = ofdma_target->of_dma_xlate(&dma_spec_target, ofdma_target);
>> + if (chan) {
>> + chan->router = ofdma->dma_router;
>> + chan->route_data = route_data;
>> + } else {
>> + ofdma->dma_router->route_free(ofdma->dma_router->dev, route_data);
>
> This line is over 80 characters.
Oh, true.
>> +int of_dma_router_register(struct device_node *np,
>> + void *(*of_dma_route_allocate)
>> + (struct of_phandle_args *, struct of_dma *),
>> + struct dma_router *dma_router)
>> +{
>> + struct of_dma *ofdma;
>> +
>> + if (!np || !of_dma_route_allocate || !dma_router) {
>> + pr_err("%s: not enough information provided\n", __func__);
>> + return -EINVAL;
>> + }
>> +
>> + ofdma = kzalloc(sizeof(*ofdma), GFP_KERNEL);
>> + if (!ofdma)
>> + return -ENOMEM;
>
> Is that expected that you allocate through kzalloc, but never have a
> matching free function implemented?
The free is via the of_dma_router_free() in case the router is removed
runtime, which is unlikely to happen since it will cause all DMA request to fail.
>> diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h
>> index 56bc026c143f..734e449f87c1 100644
>> --- a/include/linux/of_dma.h
>> +++ b/include/linux/of_dma.h
>> @@ -23,6 +23,9 @@ struct of_dma {
>> struct device_node *of_node;
>> struct dma_chan *(*of_dma_xlate)
>> (struct of_phandle_args *, struct of_dma *);
>> + void *(*of_dma_route_allocate)
>> + (struct of_phandle_args *, struct of_dma *);
>> + struct dma_router *dma_router;
>
> I don't really see why this is really tied to the device tree.
The signal router is not a DMA device, it is represented in the device tree
and the code will do the needed translation, which is transparent for the DMA
clients and also for the DMA controllers. Neither should know about the signal
router.
Similar translation can be done for ACPI.
> Couldn't we use the device_alloc_chan_resources to do that?
Not really. The router itself is not a DMA controller. The routing need to be
configured before the device_alloc_chan_resources can be called for the real
DMA controller. The signal router (core part + the HW driver) need to prepare
the route and do the translation so the filter function of the DMA driver can
validate the translated request.
--
P?ter
On Thu, Apr 09, 2015 at 11:24:58AM +0300, Peter Ujfalusi wrote:
> On 04/08/2015 06:42 PM, Maxime Ripard wrote:
> >> ---
> >> Documentation/devicetree/bindings/dma/dma.txt | 28 +++++++++
> >> drivers/dma/dmaengine.c | 7 +++
> >> drivers/dma/of-dma.c | 86 +++++++++++++++++++++++++++
> >> include/linux/dmaengine.h | 17 ++++++
> >> include/linux/of_dma.h | 21 +++++++
> >> 5 files changed, 159 insertions(+)
> >
> > Can that be moved to a header / C file of its own?
> >
> > There's a lot of various code already in dmaengine.h and dmaengine.c,
> > it would be really great to avoid adding more random stuff in there.
>
> This patch adds the core support for DMA signal routers. It adds
> fairly small amount of generic code to the core to achieve this. I
> don't think it would be better to create let's say of-dma-router.c
> and .h just for this and export functions from of-dma.c to be used
> outside of the file.
A lot of "a fairly small amount of generic code" has been added over
time, and we ended up in the current situation.
It's a bit sad if we just end up moving this just after it got merged,
especially if it doesn't have any kind of dependency on any of the
core function.
> >> +int of_dma_router_register(struct device_node *np,
> >> + void *(*of_dma_route_allocate)
> >> + (struct of_phandle_args *, struct of_dma *),
> >> + struct dma_router *dma_router)
> >> +{
> >> + struct of_dma *ofdma;
> >> +
> >> + if (!np || !of_dma_route_allocate || !dma_router) {
> >> + pr_err("%s: not enough information provided\n", __func__);
> >> + return -EINVAL;
> >> + }
> >> +
> >> + ofdma = kzalloc(sizeof(*ofdma), GFP_KERNEL);
> >> + if (!ofdma)
> >> + return -ENOMEM;
> >
> > Is that expected that you allocate through kzalloc, but never have a
> > matching free function implemented?
>
> The free is via the of_dma_router_free() in case the router is removed
> runtime, which is unlikely to happen since it will cause all DMA request to fail.
Ok.
> >> diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h
> >> index 56bc026c143f..734e449f87c1 100644
> >> --- a/include/linux/of_dma.h
> >> +++ b/include/linux/of_dma.h
> >> @@ -23,6 +23,9 @@ struct of_dma {
> >> struct device_node *of_node;
> >> struct dma_chan *(*of_dma_xlate)
> >> (struct of_phandle_args *, struct of_dma *);
> >> + void *(*of_dma_route_allocate)
> >> + (struct of_phandle_args *, struct of_dma *);
> >> + struct dma_router *dma_router;
> >
> > I don't really see why this is really tied to the device tree.
>
> The signal router is not a DMA device, it is represented in the device tree
> and the code will do the needed translation, which is transparent for the DMA
> clients and also for the DMA controllers. Neither should know about the signal
> router.
Yeah, I got that part, and we do agree on that.
> Similar translation can be done for ACPI.
But this argument is exactly why it shouldn't be tied to the device
tree. We wouldn't like to re-do all this all over again for ACPI,
while your code seems to just handle that very well, wouldn't we?
> > Couldn't we use the device_alloc_chan_resources to do that?
>
> Not really. The router itself is not a DMA controller. The routing
> need to be configured before the device_alloc_chan_resources can be
> called for the real DMA controller. The signal router (core part +
> the HW driver) need to prepare the route and do the translation so
> the filter function of the DMA driver can validate the translated
> request.
Ah, yes, hence why you need a custom xlate function. Got it, thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
On 04/10/2015 10:40 AM, Maxime Ripard wrote:
> On Thu, Apr 09, 2015 at 11:24:58AM +0300, Peter Ujfalusi wrote:
>> On 04/08/2015 06:42 PM, Maxime Ripard wrote:
>>>> ---
>>>> Documentation/devicetree/bindings/dma/dma.txt | 28 +++++++++
>>>> drivers/dma/dmaengine.c | 7 +++
>>>> drivers/dma/of-dma.c | 86 +++++++++++++++++++++++++++
>>>> include/linux/dmaengine.h | 17 ++++++
>>>> include/linux/of_dma.h | 21 +++++++
>>>> 5 files changed, 159 insertions(+)
>>>
>>> Can that be moved to a header / C file of its own?
>>>
>>> There's a lot of various code already in dmaengine.h and dmaengine.c,
>>> it would be really great to avoid adding more random stuff in there.
>>
>> This patch adds the core support for DMA signal routers. It adds
>> fairly small amount of generic code to the core to achieve this. I
>> don't think it would be better to create let's say of-dma-router.c
>> and .h just for this and export functions from of-dma.c to be used
>> outside of the file.
>
> A lot of "a fairly small amount of generic code" has been added over
> time, and we ended up in the current situation.
So that we have fairly good dmaengine core? If people using the code, they
will eventually fix things, add new features. When dmaengine came to existence
we did not had DMA routers, now that the SoCs are getting even more
complicated they are coming (in case of DRA7x, it is already here).
> It's a bit sad if we just end up moving this just after it got merged,
> especially if it doesn't have any kind of dependency on any of the
> core function.
This provides core functionality to handle DMA signal routers via DT. It does
depend on the DT DMA parsing core code as it integrates with it.
>> The signal router is not a DMA device, it is represented in the device tree
>> and the code will do the needed translation, which is transparent for the DMA
>> clients and also for the DMA controllers. Neither should know about the signal
>> router.
>
> Yeah, I got that part, and we do agree on that.
>
>> Similar translation can be done for ACPI.
>
> But this argument is exactly why it shouldn't be tied to the device
> tree. We wouldn't like to re-do all this all over again for ACPI,
> while your code seems to just handle that very well, wouldn't we?
The data structures used by the DT and ACPI layers are different. Underneath
DT and ACPI we do not have enough information on how to handle the DMA signal
router. If you happen to have multiple DMA controllers in the system for
example how would you tell how the routers are connected?
In DRA7x we have sDMA and eDMA controllers. Both have similar DMA crossbar. It
is only possible to know the hierarchy of the components via DT. I'm sure this
is the same for ACPI. You take away the ACPI layer and you loose the topology
of the system.
Probably with huge architectural change in the dmaengine core (and thus with
all the drivers) this might be possible, but I don't think that is a valid
route to take.
--
P?ter