[v1,22/43] dma: cirrus: add DT support for Cirrus EP93xx
Commit Message
- find register range from the device tree
- get clocks, interrupts from device tree
Co-developed-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---
arch/arm/mach-ep93xx/dma.c | 1 +
drivers/dma/ep93xx_dma.c | 136 +++++++++++++++++++++--
include/linux/platform_data/dma-ep93xx.h | 3 +
3 files changed, 130 insertions(+), 10 deletions(-)
Comments
Thu, Jun 01, 2023 at 08:45:27AM +0300, Nikita Shubin kirjoitti:
> - find register range from the device tree
> - get clocks, interrupts from device tree
...
> --- a/arch/arm/mach-ep93xx/dma.c
> +++ b/arch/arm/mach-ep93xx/dma.c
> @@ -19,6 +19,7 @@
> #include <linux/init.h>
> #include <linux/interrupt.h>
> #include <linux/kernel.h>
> +#include <linux/of.h>
> #include <linux/platform_device.h>
>
> #include <linux/platform_data/dma-ep93xx.h>
Stray change.
...
> --- a/drivers/dma/ep93xx_dma.c
> +++ b/drivers/dma/ep93xx_dma.c
> @@ -20,6 +20,7 @@
> #include <linux/dmaengine.h>
> #include <linux/module.h>
> #include <linux/mod_devicetable.h>
> +#include <linux/of_device.h>
> #include <linux/platform_device.h>
> #include <linux/slab.h>
...
> - struct ep93xx_dma_chan channels[];
> + struct ep93xx_dma_chan *channels;
Why? This is helpful to allocate main structure and channels in one go.
...
> @@ -875,9 +881,11 @@ static int ep93xx_dma_alloc_chan_resources(struct dma_chan *chan)
> if (!edmac->edma->m2m) {
> if (!data)
> return -EINVAL;
> +
> if (data->port < EP93XX_DMA_I2S1 ||
> data->port > EP93XX_DMA_IRDA)
> return -EINVAL;
> +
> if (data->direction != ep93xx_dma_chan_direction(chan))
> return -EINVAL;
> } else {
Seems unrelated change.
> ep93xx_dma_advance_work(to_ep93xx_dma_chan(chan));
> }
...
> +static const struct of_device_id ep93xx_dma_of_ids[] = {
> + { .compatible = "cirrus,ep9301-dma-m2p", .data = &edma_m2p },
> + { .compatible = "cirrus,ep9301-dma-m2m", .data = &edma_m2m },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, ep93xx_dma_of_ids);
Move this closer to the real user (see below).
Also this ID table shouldn't be under ifdeffery.
...
> + struct device_node *np = pdev->dev.of_node;
> + const struct of_device_id *match = of_match_node(ep93xx_dma_of_ids, pdev->dev.of_node);
> + const struct ep93xx_edma_data *data = match->data;
No NULL check? Why do you have this duplication, btw?
> + struct dma_device *dma_dev = &edma->dma_dev;
> + int num_channels;
> + int i;
> +
> + match = of_match_device((ep93xx_dma_of_ids), &pdev->dev);
> + if (!match || !match->data) {
> + dev_err(&pdev->dev, "No device match found\n");
> + return -ENODEV;
> + }
Use of_device_get_match_data().
...
> + edma->channels = devm_kzalloc(&pdev->dev,
> + num_channels * sizeof(struct ep93xx_dma_chan),
> + GFP_KERNEL);
> + if (!edma->channels)
> + return -ENOMEM;
Simply no. See below what to do.
...
> + for (i = 0; i < num_channels; i++) {
> + struct ep93xx_dma_chan *edmac = &edma->channels[i];
> +
> + edmac->chan.device = dma_dev;
> + edmac->regs = devm_platform_ioremap_resource(pdev, i);
No error check?
> + edmac->irq = platform_get_irq(pdev, i);
No error check?
> + edmac->edma = edma;
> +
> + edmac->clk = of_clk_get(np, i);
Can this actually use clk_get() or its devm_*() variant?
> +
Redundant blank line.
> + if (IS_ERR(edmac->clk)) {
> + dev_warn(&pdev->dev, "failed to get clock\n");
> + continue;
> + }
> +
> + spin_lock_init(&edmac->lock);
> + INIT_LIST_HEAD(&edmac->active);
> + INIT_LIST_HEAD(&edmac->queue);
> + INIT_LIST_HEAD(&edmac->free_list);
> + tasklet_setup(&edmac->tasklet, ep93xx_dma_tasklet);
> +
> + list_add_tail(&edmac->chan.device_node,
> + &dma_dev->channels);
> + }
...
> - edma_size = pdata->num_channels * sizeof(struct ep93xx_dma_chan);
> - edma = kzalloc(sizeof(*edma) + edma_size, GFP_KERNEL);
> - if (!edma)
> + edma->channels = devm_kzalloc(&pdev->dev,
> + pdata->num_channels * sizeof(struct ep93xx_dma_chan),
> + GFP_KERNEL);
> + if (!edma->channels)
> return -ENOMEM;
No. Just include overflow.h and use struct_size().
...
> + edma = devm_kzalloc(&pdev->dev, sizeof(*edma), GFP_KERNEL);
> +
No error check?!
...
> + if (platform_get_device_id(pdev))
> + ret = ep93xx_init_from_pdata(pdev, edma);
> + else
> + ret = ep93xx_dma_of_probe(pdev, edma);
> +
Redundant blank line.
> + if (ret)
> + return ret;
@@ -19,6 +19,7 @@
#include <linux/init.h>
#include <linux/interrupt.h>
#include <linux/kernel.h>
+#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/platform_data/dma-ep93xx.h>
@@ -20,6 +20,7 @@
#include <linux/dmaengine.h>
#include <linux/module.h>
#include <linux/mod_devicetable.h>
+#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
@@ -104,6 +105,11 @@
#define DMA_MAX_CHAN_BYTES 0xffff
#define DMA_MAX_CHAN_DESCRIPTORS 32
+enum ep93xx_dma_type {
+ M2P_DMA,
+ M2M_DMA,
+};
+
struct ep93xx_dma_engine;
static int ep93xx_dma_slave_config_write(struct dma_chan *chan,
enum dma_transfer_direction dir,
@@ -213,7 +219,7 @@ struct ep93xx_dma_engine {
#define INTERRUPT_NEXT_BUFFER 2
size_t num_channels;
- struct ep93xx_dma_chan channels[];
+ struct ep93xx_dma_chan *channels;
};
static inline struct device *chan2dev(struct ep93xx_dma_chan *edmac)
@@ -875,9 +881,11 @@ static int ep93xx_dma_alloc_chan_resources(struct dma_chan *chan)
if (!edmac->edma->m2m) {
if (!data)
return -EINVAL;
+
if (data->port < EP93XX_DMA_I2S1 ||
data->port > EP93XX_DMA_IRDA)
return -EINVAL;
+
if (data->direction != ep93xx_dma_chan_direction(chan))
return -EINVAL;
} else {
@@ -1315,20 +1323,105 @@ static void ep93xx_dma_issue_pending(struct dma_chan *chan)
ep93xx_dma_advance_work(to_ep93xx_dma_chan(chan));
}
-static int __init ep93xx_dma_probe(struct platform_device *pdev)
+
+#ifdef CONFIG_OF
+struct ep93xx_edma_data {
+ u32 id;
+ size_t num_channels;
+};
+
+static const struct ep93xx_edma_data edma_m2p = {
+ .id = M2P_DMA,
+ .num_channels = 10,
+};
+
+static const struct ep93xx_edma_data edma_m2m = {
+ .id = M2M_DMA,
+ .num_channels = 2,
+};
+
+static const struct of_device_id ep93xx_dma_of_ids[] = {
+ { .compatible = "cirrus,ep9301-dma-m2p", .data = &edma_m2p },
+ { .compatible = "cirrus,ep9301-dma-m2m", .data = &edma_m2m },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, ep93xx_dma_of_ids);
+
+static int ep93xx_dma_of_probe(struct platform_device *pdev,
+ struct ep93xx_dma_engine *edma)
+{
+ struct device_node *np = pdev->dev.of_node;
+ const struct of_device_id *match = of_match_node(ep93xx_dma_of_ids, pdev->dev.of_node);
+ const struct ep93xx_edma_data *data = match->data;
+ struct dma_device *dma_dev = &edma->dma_dev;
+ int num_channels;
+ int i;
+
+ match = of_match_device((ep93xx_dma_of_ids), &pdev->dev);
+ if (!match || !match->data) {
+ dev_err(&pdev->dev, "No device match found\n");
+ return -ENODEV;
+ }
+
+ edma->m2m = data->id;
+ num_channels = data->num_channels;
+ edma->channels = devm_kzalloc(&pdev->dev,
+ num_channels * sizeof(struct ep93xx_dma_chan),
+ GFP_KERNEL);
+ if (!edma->channels)
+ return -ENOMEM;
+
+ edma->num_channels = num_channels;
+
+ INIT_LIST_HEAD(&dma_dev->channels);
+ for (i = 0; i < num_channels; i++) {
+ struct ep93xx_dma_chan *edmac = &edma->channels[i];
+
+ edmac->chan.device = dma_dev;
+ edmac->regs = devm_platform_ioremap_resource(pdev, i);
+ edmac->irq = platform_get_irq(pdev, i);
+ edmac->edma = edma;
+
+ edmac->clk = of_clk_get(np, i);
+
+ if (IS_ERR(edmac->clk)) {
+ dev_warn(&pdev->dev, "failed to get clock\n");
+ continue;
+ }
+
+ spin_lock_init(&edmac->lock);
+ INIT_LIST_HEAD(&edmac->active);
+ INIT_LIST_HEAD(&edmac->queue);
+ INIT_LIST_HEAD(&edmac->free_list);
+ tasklet_setup(&edmac->tasklet, ep93xx_dma_tasklet);
+
+ list_add_tail(&edmac->chan.device_node,
+ &dma_dev->channels);
+ }
+
+ return 0;
+}
+#else
+static int ep93xx_dma_of_probe(struct platform_device *pdev,
+ struct ep93xx_dma_engine *edma)
+{
+ return -EINVAL;
+}
+#endif
+
+static int ep93xx_init_from_pdata(struct platform_device *pdev,
+ struct ep93xx_dma_engine *edma)
{
struct ep93xx_dma_platform_data *pdata = dev_get_platdata(&pdev->dev);
- struct ep93xx_dma_engine *edma;
- struct dma_device *dma_dev;
- size_t edma_size;
- int ret, i;
+ struct dma_device *dma_dev = &edma->dma_dev;
+ int i;
- edma_size = pdata->num_channels * sizeof(struct ep93xx_dma_chan);
- edma = kzalloc(sizeof(*edma) + edma_size, GFP_KERNEL);
- if (!edma)
+ edma->channels = devm_kzalloc(&pdev->dev,
+ pdata->num_channels * sizeof(struct ep93xx_dma_chan),
+ GFP_KERNEL);
+ if (!edma->channels)
return -ENOMEM;
- dma_dev = &edma->dma_dev;
edma->m2m = platform_get_device_id(pdev)->driver_data;
edma->num_channels = pdata->num_channels;
@@ -1359,6 +1452,27 @@ static int __init ep93xx_dma_probe(struct platform_device *pdev)
&dma_dev->channels);
}
+ return 0;
+}
+
+static int __init ep93xx_dma_probe(struct platform_device *pdev)
+{
+ struct ep93xx_dma_engine *edma;
+ struct dma_device *dma_dev;
+ int ret, i;
+
+ edma = devm_kzalloc(&pdev->dev, sizeof(*edma), GFP_KERNEL);
+
+ if (platform_get_device_id(pdev))
+ ret = ep93xx_init_from_pdata(pdev, edma);
+ else
+ ret = ep93xx_dma_of_probe(pdev, edma);
+
+ if (ret)
+ return ret;
+
+ dma_dev = &edma->dma_dev;
+
dma_cap_zero(dma_dev->cap_mask);
dma_cap_set(DMA_SLAVE, dma_dev->cap_mask);
dma_cap_set(DMA_CYCLIC, dma_dev->cap_mask);
@@ -1415,10 +1529,12 @@ static const struct platform_device_id ep93xx_dma_driver_ids[] = {
{ "ep93xx-dma-m2m", 1 },
{ },
};
+MODULE_DEVICE_TABLE(of, ep93xx_dma_driver_ids);
static struct platform_driver ep93xx_dma_driver = {
.driver = {
.name = "ep93xx-dma",
+ .of_match_table = ep93xx_dma_of_ids,
},
.id_table = ep93xx_dma_driver_ids,
};
@@ -70,6 +70,9 @@ struct ep93xx_dma_platform_data {
static inline bool ep93xx_dma_chan_is_m2p(struct dma_chan *chan)
{
+ if (of_device_is_compatible(dev_of_node(chan->device->dev), "cirrus,ep9301-dma-m2p"))
+ return true;
+
return !strcmp(dev_name(chan->device->dev), "ep93xx-dma-m2p");
}