Re: [PATCH v9 09/38] dma: cirrus: Convert to DT for Cirrus EP93xx
From: Vinod Koul
Date: Thu Mar 28 2024 - 00:32:05 EST
On 26-03-24, 12:18, Nikita Shubin via B4 Relay wrote:
> From: Nikita Shubin <nikita.shubin@xxxxxxxxxxx>
>
> Convert Cirrus EP93xx DMA to device tree usage:
Subsytem is dmaengine: pls fix that
>
> - add OF ID match table with data
> - add of_probe for device tree
> - add xlate for m2m/m2p
> - drop subsys_initcall code
> - drop platform probe
> - drop platform structs usage
>
> >From now on it only supports device tree probing.
>
> Co-developed-by: Alexander Sverdlin <alexander.sverdlin@xxxxxxxxx>
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@xxxxxxxxx>
> Signed-off-by: Nikita Shubin <nikita.shubin@xxxxxxxxxxx>
> ---
> drivers/dma/ep93xx_dma.c | 239 ++++++++++++++++++++++++-------
> include/linux/platform_data/dma-ep93xx.h | 6 +
> 2 files changed, 191 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/dma/ep93xx_dma.c b/drivers/dma/ep93xx_dma.c
> index d6c60635e90d..17c8e2badee2 100644
> --- a/drivers/dma/ep93xx_dma.c
> +++ b/drivers/dma/ep93xx_dma.c
> @@ -20,6 +20,8 @@
> #include <linux/dmaengine.h>
> #include <linux/module.h>
> #include <linux/mod_devicetable.h>
> +#include <linux/of_dma.h>
> +#include <linux/overflow.h>
> #include <linux/platform_device.h>
> #include <linux/slab.h>
>
> @@ -104,6 +106,11 @@
> #define DMA_MAX_CHAN_BYTES 0xffff
> #define DMA_MAX_CHAN_DESCRIPTORS 32
>
> +enum ep93xx_dma_type {
> + M2P_DMA,
Is this missing P2M?
> + M2M_DMA,
> +};
> +
> struct ep93xx_dma_engine;
> static int ep93xx_dma_slave_config_write(struct dma_chan *chan,
> enum dma_transfer_direction dir,
> @@ -129,11 +136,17 @@ struct ep93xx_dma_desc {
> struct list_head node;
> };
>
> +struct ep93xx_dma_chan_cfg {
> + u8 port;
> + enum dma_transfer_direction dir;
Why is direction stored here, it should be derived from the prep_xxx
call, that has direction as an argument
> +};
> +
> /**
> * struct ep93xx_dma_chan - an EP93xx DMA M2P/M2M channel
> * @chan: dmaengine API channel
> * @edma: pointer to the engine device
> * @regs: memory mapped registers
> + * @dma_cfg: channel number, direction
> * @irq: interrupt number of the channel
> * @clk: clock used by this channel
> * @tasklet: channel specific tasklet used for callbacks
> @@ -157,14 +170,12 @@ struct ep93xx_dma_desc {
> * descriptor in the chain. When a descriptor is moved to the @active queue,
> * the first and chained descriptors are flattened into a single list.
> *
> - * @chan.private holds pointer to &struct ep93xx_dma_data which contains
> - * necessary channel configuration information. For memcpy channels this must
> - * be %NULL.
> */
> struct ep93xx_dma_chan {
> struct dma_chan chan;
> const struct ep93xx_dma_engine *edma;
> void __iomem *regs;
> + struct ep93xx_dma_chan_cfg dma_cfg;
> int irq;
> struct clk *clk;
> struct tasklet_struct tasklet;
> @@ -216,6 +227,11 @@ struct ep93xx_dma_engine {
> struct ep93xx_dma_chan channels[] __counted_by(num_channels);
> };
>
> +struct ep93xx_edma_data {
> + u32 id;
> + size_t num_channels;
> +};
> +
> static inline struct device *chan2dev(struct ep93xx_dma_chan *edmac)
> {
> return &edmac->chan.dev->device;
> @@ -318,10 +334,9 @@ static void m2p_set_control(struct ep93xx_dma_chan *edmac, u32 control)
>
> static int m2p_hw_setup(struct ep93xx_dma_chan *edmac)
> {
> - struct ep93xx_dma_data *data = edmac->chan.private;
> u32 control;
>
> - writel(data->port & 0xf, edmac->regs + M2P_PPALLOC);
> + writel(edmac->dma_cfg.port & 0xf, edmac->regs + M2P_PPALLOC);
>
> control = M2P_CONTROL_CH_ERROR_INT | M2P_CONTROL_ICE
> | M2P_CONTROL_ENABLE;
> @@ -458,16 +473,15 @@ static int m2p_hw_interrupt(struct ep93xx_dma_chan *edmac)
>
> static int m2m_hw_setup(struct ep93xx_dma_chan *edmac)
> {
> - const struct ep93xx_dma_data *data = edmac->chan.private;
> u32 control = 0;
>
> - if (!data) {
> + if (edmac->dma_cfg.dir == DMA_MEM_TO_MEM) {
> /* This is memcpy channel, nothing to configure */
> writel(control, edmac->regs + M2M_CONTROL);
> return 0;
> }
>
> - switch (data->port) {
> + switch (edmac->dma_cfg.port) {
> case EP93XX_DMA_SSP:
> /*
> * This was found via experimenting - anything less than 5
> @@ -477,7 +491,7 @@ static int m2m_hw_setup(struct ep93xx_dma_chan *edmac)
> control = (5 << M2M_CONTROL_PWSC_SHIFT);
> control |= M2M_CONTROL_NO_HDSK;
>
> - if (data->direction == DMA_MEM_TO_DEV) {
> + if (edmac->dma_cfg.dir == DMA_MEM_TO_DEV) {
> control |= M2M_CONTROL_DAH;
> control |= M2M_CONTROL_TM_TX;
> control |= M2M_CONTROL_RSS_SSPTX;
> @@ -493,7 +507,7 @@ static int m2m_hw_setup(struct ep93xx_dma_chan *edmac)
> * This IDE part is totally untested. Values below are taken
> * from the EP93xx Users's Guide and might not be correct.
> */
> - if (data->direction == DMA_MEM_TO_DEV) {
> + if (edmac->dma_cfg.dir == DMA_MEM_TO_DEV) {
> /* Worst case from the UG */
> control = (3 << M2M_CONTROL_PWSC_SHIFT);
> control |= M2M_CONTROL_DAH;
> @@ -548,7 +562,6 @@ static void m2m_fill_desc(struct ep93xx_dma_chan *edmac)
>
> static void m2m_hw_submit(struct ep93xx_dma_chan *edmac)
> {
> - struct ep93xx_dma_data *data = edmac->chan.private;
> u32 control = readl(edmac->regs + M2M_CONTROL);
>
> /*
> @@ -574,7 +587,7 @@ static void m2m_hw_submit(struct ep93xx_dma_chan *edmac)
> control |= M2M_CONTROL_ENABLE;
> writel(control, edmac->regs + M2M_CONTROL);
>
> - if (!data) {
> + if (edmac->dma_cfg.dir == DMA_MEM_TO_MEM) {
> /*
> * For memcpy channels the software trigger must be asserted
> * in order to start the memcpy operation.
> @@ -636,7 +649,7 @@ static int m2m_hw_interrupt(struct ep93xx_dma_chan *edmac)
> */
> if (ep93xx_dma_advance_active(edmac)) {
> m2m_fill_desc(edmac);
> - if (done && !edmac->chan.private) {
> + if (done && edmac->dma_cfg.dir == DMA_MEM_TO_MEM) {
> /* Software trigger for memcpy channel */
> control = readl(edmac->regs + M2M_CONTROL);
> control |= M2M_CONTROL_START;
> @@ -867,25 +880,22 @@ static dma_cookie_t ep93xx_dma_tx_submit(struct dma_async_tx_descriptor *tx)
> static int ep93xx_dma_alloc_chan_resources(struct dma_chan *chan)
> {
> struct ep93xx_dma_chan *edmac = to_ep93xx_dma_chan(chan);
> - struct ep93xx_dma_data *data = chan->private;
> const char *name = dma_chan_name(chan);
> int ret, i;
>
> /* Sanity check the channel parameters */
> if (!edmac->edma->m2m) {
> - if (!data)
> + if (edmac->dma_cfg.port < EP93XX_DMA_I2S1 ||
> + edmac->dma_cfg.port > EP93XX_DMA_IRDA)
> return -EINVAL;
> - if (data->port < EP93XX_DMA_I2S1 ||
> - data->port > EP93XX_DMA_IRDA)
> - return -EINVAL;
> - if (data->direction != ep93xx_dma_chan_direction(chan))
> + if (edmac->dma_cfg.dir != ep93xx_dma_chan_direction(chan))
> return -EINVAL;
> } else {
> - if (data) {
> - switch (data->port) {
> + if (edmac->dma_cfg.dir != DMA_MEM_TO_MEM) {
> + switch (edmac->dma_cfg.port) {
> case EP93XX_DMA_SSP:
> case EP93XX_DMA_IDE:
> - if (!is_slave_direction(data->direction))
> + if (!is_slave_direction(edmac->dma_cfg.dir))
> return -EINVAL;
> break;
> default:
> @@ -894,9 +904,6 @@ static int ep93xx_dma_alloc_chan_resources(struct dma_chan *chan)
> }
> }
>
> - if (data && data->name)
> - name = data->name;
> -
> ret = clk_prepare_enable(edmac->clk);
> if (ret)
> return ret;
> @@ -1315,36 +1322,53 @@ 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)
> +static struct ep93xx_dma_engine *ep93xx_dma_of_probe(struct platform_device *pdev)
> {
> - struct ep93xx_dma_platform_data *pdata = dev_get_platdata(&pdev->dev);
> + const struct ep93xx_edma_data *data;
> + struct device *dev = &pdev->dev;
> struct ep93xx_dma_engine *edma;
> struct dma_device *dma_dev;
> - int ret, i;
> + char dma_clk_name[5];
> + int i;
>
> - edma = kzalloc(struct_size(edma, channels, pdata->num_channels), GFP_KERNEL);
> + data = device_get_match_data(dev);
> + if (!data)
> + return ERR_PTR(dev_err_probe(dev, -ENODEV, "No device match found\n"));
> +
> + edma = devm_kzalloc(dev, struct_size(edma, channels, data->num_channels),
> + GFP_KERNEL);
> if (!edma)
> - return -ENOMEM;
> + return ERR_PTR(-ENOMEM);
>
> + edma->m2m = data->id;
> + edma->num_channels = data->num_channels;
> dma_dev = &edma->dma_dev;
> - edma->m2m = platform_get_device_id(pdev)->driver_data;
> - edma->num_channels = pdata->num_channels;
>
> INIT_LIST_HEAD(&dma_dev->channels);
> - for (i = 0; i < pdata->num_channels; i++) {
> - const struct ep93xx_dma_chan_data *cdata = &pdata->channels[i];
> + for (i = 0; i < edma->num_channels; i++) {
> struct ep93xx_dma_chan *edmac = &edma->channels[i];
>
> edmac->chan.device = dma_dev;
> - edmac->regs = cdata->base;
> - edmac->irq = cdata->irq;
> + edmac->regs = devm_platform_ioremap_resource(pdev, i);
> + if (IS_ERR(edmac->regs))
> + return edmac->regs;
> +
> + edmac->irq = fwnode_irq_get(dev_fwnode(dev), i);
> + if (edmac->irq < 0)
> + return ERR_PTR(edmac->irq);
> +
> edmac->edma = edma;
>
> - edmac->clk = clk_get(NULL, cdata->name);
> + if (edma->m2m)
> + sprintf(dma_clk_name, "m2m%u", i);
> + else
> + sprintf(dma_clk_name, "m2p%u", i);
> +
> + edmac->clk = devm_clk_get(dev, dma_clk_name);
> if (IS_ERR(edmac->clk)) {
> - dev_warn(&pdev->dev, "failed to get clock for %s\n",
> - cdata->name);
> - continue;
> + dev_err_probe(dev, PTR_ERR(edmac->clk),
> + "no %s clock found\n", dma_clk_name);
> + return ERR_CAST(edmac->clk);
> }
>
> spin_lock_init(&edmac->lock);
> @@ -1357,6 +1381,90 @@ static int __init ep93xx_dma_probe(struct platform_device *pdev)
> &dma_dev->channels);
> }
>
> + return edma;
> +}
> +
> +static bool ep93xx_m2p_dma_filter(struct dma_chan *chan, void *filter_param)
> +{
> + struct ep93xx_dma_chan *echan = to_ep93xx_dma_chan(chan);
> + struct ep93xx_dma_chan_cfg *cfg = filter_param;
> +
> + if (cfg->dir != ep93xx_dma_chan_direction(chan))
> + return false;
> +
> + echan->dma_cfg = *cfg;
> + return true;
> +}
> +
> +static struct dma_chan *ep93xx_m2p_dma_of_xlate(struct of_phandle_args *dma_spec,
> + struct of_dma *ofdma)
> +{
> + struct ep93xx_dma_engine *edma = ofdma->of_dma_data;
> + dma_cap_mask_t mask = edma->dma_dev.cap_mask;
> + struct ep93xx_dma_chan_cfg dma_cfg;
> + u8 port = dma_spec->args[0];
> + u8 direction = dma_spec->args[1];
> +
> + if (port > EP93XX_DMA_IRDA)
> + return NULL;
> +
> + if (!is_slave_direction(direction))
> + return NULL;
> +
> + dma_cfg.port = port;
> + dma_cfg.dir = direction;
> +
> + return __dma_request_channel(&mask, ep93xx_m2p_dma_filter, &dma_cfg, ofdma->of_node);
> +}
> +
> +static bool ep93xx_m2m_dma_filter(struct dma_chan *chan, void *filter_param)
> +{
> + struct ep93xx_dma_chan *echan = to_ep93xx_dma_chan(chan);
> + struct ep93xx_dma_chan_cfg *cfg = filter_param;
> +
> + echan->dma_cfg = *cfg;
> +
> + return true;
> +}
> +
> +static struct dma_chan *ep93xx_m2m_dma_of_xlate(struct of_phandle_args *dma_spec,
> + struct of_dma *ofdma)
> +{
> + struct ep93xx_dma_engine *edma = ofdma->of_dma_data;
> + dma_cap_mask_t mask = edma->dma_dev.cap_mask;
> + struct ep93xx_dma_chan_cfg dma_cfg;
> + u8 port = dma_spec->args[0];
> + u8 direction = dma_spec->args[1];
> +
> + if (!is_slave_direction(direction))
> + return NULL;
> +
> + switch (port) {
> + case EP93XX_DMA_SSP:
> + case EP93XX_DMA_IDE:
> + break;
> + default:
> + return NULL;
> + }
> +
> + dma_cfg.port = port;
> + dma_cfg.dir = direction;
> +
> + return __dma_request_channel(&mask, ep93xx_m2m_dma_filter, &dma_cfg, ofdma->of_node);
> +}
> +
> +static int ep93xx_dma_probe(struct platform_device *pdev)
> +{
> + struct ep93xx_dma_engine *edma;
> + struct dma_device *dma_dev;
> + int ret;
> +
> + edma = ep93xx_dma_of_probe(pdev);
> + if (!edma)
> + return PTR_ERR(edma);
> +
> + 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);
> @@ -1393,21 +1501,46 @@ static int __init ep93xx_dma_probe(struct platform_device *pdev)
> }
>
> ret = dma_async_device_register(dma_dev);
> - if (unlikely(ret)) {
> - for (i = 0; i < edma->num_channels; i++) {
> - struct ep93xx_dma_chan *edmac = &edma->channels[i];
> - if (!IS_ERR_OR_NULL(edmac->clk))
> - clk_put(edmac->clk);
> - }
> - kfree(edma);
> + if (ret)
> + return ret;
> +
> + if (edma->m2m) {
> + ret = of_dma_controller_register(pdev->dev.of_node, ep93xx_m2m_dma_of_xlate,
> + edma);
> } else {
> - dev_info(dma_dev->dev, "EP93xx M2%s DMA ready\n",
> - edma->m2m ? "M" : "P");
> + ret = of_dma_controller_register(pdev->dev.of_node, ep93xx_m2p_dma_of_xlate,
> + edma);
> }
> + if (ret)
> + goto err_dma_unregister;
> +
> + dev_info(dma_dev->dev, "EP93xx M2%s DMA ready\n", edma->m2m ? "M" : "P");
> +
> + return 0;
> +
> +err_dma_unregister:
> + dma_async_device_unregister(dma_dev);
>
> return ret;
> }
>
> +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 const struct platform_device_id ep93xx_dma_driver_ids[] = {
> { "ep93xx-dma-m2p", 0 },
> { "ep93xx-dma-m2m", 1 },
> @@ -1417,15 +1550,13 @@ static const struct platform_device_id 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,
> + .probe = ep93xx_dma_probe,
> };
>
> -static int __init ep93xx_dma_module_init(void)
> -{
> - return platform_driver_probe(&ep93xx_dma_driver, ep93xx_dma_probe);
> -}
> -subsys_initcall(ep93xx_dma_module_init);
> +module_platform_driver(ep93xx_dma_driver);
>
> MODULE_AUTHOR("Mika Westerberg <mika.westerberg@xxxxxx>");
> MODULE_DESCRIPTION("EP93xx DMA driver");
> diff --git a/include/linux/platform_data/dma-ep93xx.h b/include/linux/platform_data/dma-ep93xx.h
> index eb9805bb3fe8..9ec5cdd5a1eb 100644
> --- a/include/linux/platform_data/dma-ep93xx.h
> +++ b/include/linux/platform_data/dma-ep93xx.h
> @@ -3,8 +3,11 @@
> #define __ASM_ARCH_DMA_H
>
> #include <linux/types.h>
> +#include <linux/device.h>
> #include <linux/dmaengine.h>
> #include <linux/dma-mapping.h>
> +#include <linux/property.h>
> +#include <linux/string.h>
>
> /*
> * M2P channels.
> @@ -70,6 +73,9 @@ struct ep93xx_dma_platform_data {
>
> static inline bool ep93xx_dma_chan_is_m2p(struct dma_chan *chan)
> {
> + if (device_is_compatible(chan->device->dev, "cirrus,ep9301-dma-m2p"))
> + return true;
> +
> return !strcmp(dev_name(chan->device->dev), "ep93xx-dma-m2p");
> }
>
>
> --
> 2.41.0
>
--
~Vinod