Re: [PATCH v3 15/18] dmaengine: ste_dma40: Use power domain for LCLA SRAM

[Frank Li <Frank.li@oss.nxp.com>]

On Thu, Sep 24, 2026 at 03:26:47PM +0200, Ulf Hansson wrote:
> On Mon, Sep 21, 2026 at 12:49 AM Linus Walleij <linusw@kernel.org> wrote:
> >
> > Replace the LCLA ESRAM regulator with runtime PM.
> >
> > Use the SRAM device that owns the ESRAM34 power domain.
> >
> > Hold that domain while DMA transfers are active. The DMA controller is
> > runtime PM IRQ-safe, so keep taking explicit LCLA runtime PM references
> > from the descriptor preparation path instead of relying on a runtime PM
> > device link from the atomic transfer path.
>
> A bit of an orthogonal problem, but related to the above. The irqsafe
> thing have been discussed in the past for DMA controller drivers.
>
> Would it possible to get rid of the irqsafe configuration for the dma
> controller device, altogether?
>
> I don't recall exactly, but not the entire consumer DMA API is allowed
> to be called in atomic context, but perhaps that is not sufficient to
> allow us to drop the irqsafe configuration for the DMA controller
> device?
>
> If not, I wonder if the consumer of the DMA (like an mmc controller)
> should be assigned as a consumer device (using device links and
> DL_FLAG_PM_RUNTIME) of the DMA controller device. In this way, the
> consumer (mmc) would be able to control the supplier (DMA) through the
> device link directly using runtime PM.

Yes, I support this method.

there are someworks. welcome to join discussion to make it move forward.

https://lore.kernel.org/dmaengine/aNVufDmHjLRauKYo@lizhi-Precision-Tower-5810/
https://lore.kernel.org/dmaengine/20250903-v6-16-topic-sdma-v1-9-ac7bab629e8b@pengutronix.de/

Frank

> This could potentially avoid
> the requirement of having to use irqsafe for the DMA controller
> device. Maybe someone already tried this?
>
> If the irqsafe configuration can be dropped, in some way or the other,
> this patch could become less intrusive, as all that would be needed is
> to create a device link to sram device using DL_FLAG_PM_RUNTIME.
>
> Kind regards
> Uffe
>
> >
> > Add a stateless device link so system PM keeps the DMA controller ordered
> > after the LCLA SRAM supplier.
> >
> > Suggested-by: Frank Li <Frank.li@oss.nxp.com>
> > Assisted-by: LLM
> > Signed-off-by: Linus Walleij <linusw@kernel.org>
> > ---
> >  drivers/dma/ste_dma40.c | 130 ++++++++++++++++++++++++++++++++----------------
> >  1 file changed, 88 insertions(+), 42 deletions(-)
> >
> > diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> > index eda3f91741ed..2841af5b160b 100644
> > --- a/drivers/dma/ste_dma40.c
> > +++ b/drivers/dma/ste_dma40.c
> > @@ -21,8 +21,8 @@
> >  #include <linux/of.h>
> >  #include <linux/of_address.h>
> >  #include <linux/of_dma.h>
> > +#include <linux/of_platform.h>
> >  #include <linux/amba/bus.h>
> > -#include <linux/regulator/consumer.h>
> >
> >  #include "dmaengine.h"
> >  #include "ste_dma40.h"
> > @@ -383,6 +383,7 @@ struct d40_lli_pool {
> >   * @node: List entry.
> >   * @is_in_client_list: true if the client owns this descriptor.
> >   * @cyclic: true if this is a cyclic job
> > + * @lcla_pm_active: LCLA SRAM power domain is held for this descriptor.
> >   *
> >   * This descriptor is used for both logical and physical transfers.
> >   */
> > @@ -402,6 +403,7 @@ struct d40_desc {
> >
> >         bool                             is_in_client_list;
> >         bool                             cyclic;
> > +       bool                             lcla_pm_active;
> >  };
> >
> >  /**
> > @@ -571,7 +573,8 @@ struct d40_gen_dmac {
> >   * to phy_chans entries.
> >   * @plat_data: Pointer to provided platform_data which is the driver
> >   * configuration.
> > - * @lcpa_regulator: Pointer to hold the regulator for the esram bank for lcla.
> > + * @lcla_dev: SRAM device for the ESRAM bank used by LCLA.
> > + * @lcla_link: Device link to keep system PM ordered against LCLA.
> >   * @phy_res: Vector containing all physical channels.
> >   * @lcla_pool: lcla pool settings and data.
> >   * @lcpa_base: The virtual mapped address of LCPA.
> > @@ -606,7 +609,8 @@ struct d40_base {
> >         struct d40_chan                 **lookup_log_chans;
> >         struct d40_chan                 **lookup_phy_chans;
> >         struct stedma40_platform_data    *plat_data;
> > -       struct regulator                 *lcpa_regulator;
> > +       struct device                    *lcla_dev;
> > +       struct device_link               *lcla_link;
> >         /* Physical half channels */
> >         struct d40_phy_res               *phy_res;
> >         struct d40_lcla_pool              lcla_pool;
> > @@ -628,6 +632,36 @@ static struct device *chan2dev(struct d40_chan *d40c)
> >         return &d40c->chan.dev->device;
> >  }
> >
> > +static void d40_transfer_runtime_get(struct d40_base *base)
> > +{
> > +       pm_runtime_get_sync(base->dev);
> > +}
> > +
> > +static int d40_lcla_runtime_get(struct d40_base *base)
> > +{
> > +       if (!base->lcla_dev)
> > +               return 0;
> > +
> > +       return pm_runtime_resume_and_get(base->lcla_dev);
> > +}
> > +
> > +static void d40_desc_lcla_runtime_put(struct d40_chan *d40c,
> > +                                     struct d40_desc *d40d)
> > +{
> > +       struct d40_base *base = d40c->base;
> > +
> > +       if (!d40d->lcla_pm_active)
> > +               return;
> > +
> > +       d40d->lcla_pm_active = false;
> > +       pm_runtime_put(base->lcla_dev);
> > +}
> > +
> > +static void d40_transfer_runtime_put(struct d40_base *base)
> > +{
> > +       pm_runtime_put_autosuspend(base->dev);
> > +}
> > +
> >  static bool chan_is_physical(struct d40_chan *chan)
> >  {
> >         return chan->log_num == D40_PHY_CHAN;
> > @@ -818,6 +852,7 @@ static void d40_desc_free(struct d40_chan *d40c, struct d40_desc *d40d)
> >
> >         d40_pool_lli_free(d40c, d40d);
> >         d40_lcla_free_all(d40c, d40d);
> > +       d40_desc_lcla_runtime_put(d40c, d40d);
> >         kmem_cache_free(d40c->base->desc_slab, d40d);
> >  }
> >
> > @@ -1516,7 +1551,7 @@ static struct d40_desc *d40_queue_start(struct d40_chan *d40c)
> >         if (d40d != NULL) {
> >                 if (!d40c->busy) {
> >                         d40c->busy = true;
> > -                       pm_runtime_get_sync(d40c->base->dev);
> > +                       d40_transfer_runtime_get(d40c->base);
> >                 }
> >
> >                 /* Remove from queue */
> > @@ -1535,6 +1570,7 @@ static struct d40_desc *d40_queue_start(struct d40_chan *d40c)
> >                         d40_desc_remove(d40d);
> >                         d40_desc_free(d40c, d40d);
> >                         d40c->busy = false;
> > +                       d40_transfer_runtime_put(d40c->base);
> >                         return ERR_PTR(err);
> >                 }
> >         }
> > @@ -1583,7 +1619,7 @@ static void dma_tc_handle(struct d40_chan *d40c)
> >                 if (d40_queue_start(d40c) == NULL) {
> >                         d40c->busy = false;
> >
> > -                       pm_runtime_put_autosuspend(d40c->base->dev);
> > +                       d40_transfer_runtime_put(d40c->base);
> >                 }
> >
> >                 d40_desc_remove(d40d);
> > @@ -1637,6 +1673,7 @@ static void dma_tasklet(struct tasklet_struct *t)
> >                 } else if (!d40d->is_in_client_list) {
> >                         d40_desc_remove(d40d);
> >                         d40_lcla_free_all(d40c, d40d);
> > +                       d40_desc_lcla_runtime_put(d40c, d40d);
> >                         list_add_tail(&d40d->node, &d40c->client);
> >                         d40d->is_in_client_list = true;
> >                 }
> > @@ -2067,7 +2104,7 @@ static int d40_free_dma(struct d40_chan *d40c)
> >                 d40c->base->lookup_phy_chans[phy->num] = NULL;
> >
> >         if (d40c->busy)
> > -               pm_runtime_put_autosuspend(d40c->base->dev);
> > +               d40_transfer_runtime_put(d40c->base);
> >
> >         d40c->busy = false;
> >         d40c->phy_chan = NULL;
> > @@ -2246,6 +2283,7 @@ d40_prep_sg(struct dma_chan *dchan, struct scatterlist *sg_src,
> >         dma_addr_t dst_dev_addr;
> >         struct d40_desc *desc;
> >         unsigned long flags;
> > +       bool got_lcla_pm = false;
> >         int ret;
> >
> >         if (!chan->phy_chan) {
> > @@ -2255,11 +2293,20 @@ d40_prep_sg(struct dma_chan *dchan, struct scatterlist *sg_src,
> >
> >         d40_set_runtime_config_write(dchan, &chan->slave_config, direction);
> >
> > +       ret = d40_lcla_runtime_get(chan->base);
> > +       if (ret) {
> > +               chan_err(chan, "Failed to enable LCLA power domain\n");
> > +               return NULL;
> > +       }
> > +       got_lcla_pm = !!chan->base->lcla_dev;
> > +
> >         spin_lock_irqsave(&chan->lock, flags);
> >
> >         desc = d40_prep_desc(chan, sg_src, sg_len, dma_flags);
> >         if (desc == NULL)
> >                 goto unlock;
> > +       desc->lcla_pm_active = got_lcla_pm;
> > +       got_lcla_pm = false;
> >
> >         if (sg_next(&sg_src[sg_len - 1]) == sg_src)
> >                 desc->cyclic = true;
> > @@ -2297,6 +2344,8 @@ d40_prep_sg(struct dma_chan *dchan, struct scatterlist *sg_src,
> >         d40_desc_free(chan, desc);
> >   unlock:
> >         spin_unlock_irqrestore(&chan->lock, flags);
> > +       if (got_lcla_pm)
> > +               pm_runtime_put(chan->base->lcla_dev);
> >         return NULL;
> >  }
> >
> > @@ -2628,7 +2677,7 @@ static int d40_terminate_all(struct dma_chan *chan)
> >         d40_term_all(d40c);
> >         pm_runtime_put_autosuspend(d40c->base->dev);
> >         if (d40c->busy)
> > -               pm_runtime_put_autosuspend(d40c->base->dev);
> > +               d40_transfer_runtime_put(d40c->base);
> >         d40c->busy = false;
> >
> >         spin_unlock_irqrestore(&d40c->lock, flags);
> > @@ -2931,29 +2980,11 @@ static int __init d40_dmaengine_init(struct d40_base *base,
> >  #ifdef CONFIG_PM_SLEEP
> >  static int dma40_suspend(struct device *dev)
> >  {
> > -       struct d40_base *base = dev_get_drvdata(dev);
> > -       int ret;
> > -
> > -       ret = pm_runtime_force_suspend(dev);
> > -       if (ret)
> > -               return ret;
> > -
> > -       if (base->lcpa_regulator)
> > -               ret = regulator_disable(base->lcpa_regulator);
> > -       return ret;
> > +       return pm_runtime_force_suspend(dev);
> >  }
> >
> >  static int dma40_resume(struct device *dev)
> >  {
> > -       struct d40_base *base = dev_get_drvdata(dev);
> > -       int ret = 0;
> > -
> > -       if (base->lcpa_regulator) {
> > -               ret = regulator_enable(base->lcpa_regulator);
> > -               if (ret)
> > -                       return ret;
> > -       }
> > -
> >         return pm_runtime_force_resume(dev);
> >  }
> >  #endif
> > @@ -3509,7 +3540,10 @@ static int __init d40_probe(struct platform_device *pdev)
> >         struct device *dev = &pdev->dev;
> >         struct device_node *np = pdev->dev.of_node;
> >         struct device_node *np_lcpa;
> > +       struct device_node *np_lcla;
> > +       struct device_node *np_lcla_parent;
> >         struct d40_base *base;
> > +       struct platform_device *lcla_pdev;
> >         struct resource *res;
> >         struct resource res_lcpa;
> >         int num_reserved_chans;
> > @@ -3610,21 +3644,32 @@ static int __init d40_probe(struct platform_device *pdev)
> >         irq_requested = true;
> >
> >         if (base->plat_data->use_esram_lcla) {
> > +               np_lcla = of_parse_phandle(np, "sram", 1);
> > +               if (!np_lcla) {
> > +                       dev_err(dev, "no LCLA SRAM node\n");
> > +                       ret = -EINVAL;
> > +                       goto destroy_cache;
> > +               }
> >
> > -               base->lcpa_regulator = regulator_get(base->dev, "lcla_esram");
> > -               if (IS_ERR(base->lcpa_regulator)) {
> > -                       d40_err(dev, "Failed to get lcpa_regulator\n");
> > -                       ret = PTR_ERR(base->lcpa_regulator);
> > -                       base->lcpa_regulator = NULL;
> > +               np_lcla_parent = of_get_parent(np_lcla);
> > +               of_node_put(np_lcla);
> > +               if (!np_lcla_parent) {
> > +                       dev_err(dev, "no LCLA SRAM parent node\n");
> > +                       ret = -EINVAL;
> >                         goto destroy_cache;
> >                 }
> >
> > -               ret = regulator_enable(base->lcpa_regulator);
> > -               if (ret) {
> > -                       d40_err(dev,
> > -                               "Failed to enable lcpa_regulator\n");
> > -                       regulator_put(base->lcpa_regulator);
> > -                       base->lcpa_regulator = NULL;
> > +               lcla_pdev = of_find_device_by_node(np_lcla_parent);
> > +               of_node_put(np_lcla_parent);
> > +               if (!lcla_pdev) {
> > +                       ret = -EPROBE_DEFER;
> > +                       goto destroy_cache;
> > +               }
> > +               base->lcla_dev = &lcla_pdev->dev;
> > +               base->lcla_link = device_link_add(dev, base->lcla_dev,
> > +                                                 DL_FLAG_STATELESS);
> > +               if (!base->lcla_link) {
> > +                       ret = -ENODEV;
> >                         goto destroy_cache;
> >                 }
> >         }
> > @@ -3663,16 +3708,17 @@ static int __init d40_probe(struct platform_device *pdev)
> >                                  SZ_1K * base->num_phy_chans,
> >                                  DMA_TO_DEVICE);
> >
> > -       if (!base->lcla_pool.base_unaligned && base->lcla_pool.base)
> > +       if (!base->lcla_pool.base_unaligned && base->lcla_pool.base &&
> > +           base->lcla_pool.pages)
> >                 free_pages((unsigned long)base->lcla_pool.base,
> >                            base->lcla_pool.pages);
> >
> >         kfree(base->lcla_pool.base_unaligned);
> >
> > -       if (base->lcpa_regulator) {
> > -               regulator_disable(base->lcpa_regulator);
> > -               regulator_put(base->lcpa_regulator);
> > -       }
> > +       if (base->lcla_link)
> > +               device_link_del(base->lcla_link);
> > +       if (base->lcla_dev)
> > +               put_device(base->lcla_dev);
> >         if (irq_requested)
> >                 free_irq(base->irq, base);
> >         if (runtime_pm_enabled)
> >
> > --
> > 2.55.0
> >