On Mon, Sep 21, 2026 at 12:49 AM Linus Walleij <[email protected]> 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. 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 <[email protected]> > Assisted-by: LLM > Signed-off-by: Linus Walleij <[email protected]> > --- > 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 >
