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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] dmaengine: ste_dma40: pm_runtime_resume_and_get() sleeps in atomic context
- [Medium] dmaengine: ste_dma40: unnecessary LCLA SRAM power domain wakeup for physical channels
--

commit 711af1477e65e64325b0b0970b88c4b5d746ee74
Author: Linus Walleij <linusw@kernel.org>

dmaengine: ste_dma40: Use power domain for LCLA SRAM

This patch replaces the LCLA ESRAM regulator with runtime PM handling.
It adds a reference to the SRAM device that owns the ESRAM34 power domain
and holds that domain during descriptor preparation while transfers are
active.

> diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
> index eda3f91741ed5..2841af5b160b8 100644
> --- a/drivers/dma/ste_dma40.c
> +++ b/drivers/dma/ste_dma40.c

[ ... ]

> @@ -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);

[Severity: High]
Does this sleep in atomic context?

When d40_prep_sg() in drivers/dma/ste_dma40.c prepares a transfer, it calls
d40_lcla_runtime_get(), which invokes pm_runtime_resume_and_get() here.
DMA clients can call dmaengine_prep_slave_sg() or dmaengine_prep_dma_cyclic()
while holding a spinlock or from an interrupt handler. Since lcla_dev is an
external platform device and is not marked pm_runtime_irq_safe(), the PM
core's rpm_resume() will attempt to sleep waiting for the power domain.

Could this cause a scheduling while atomic panic when clients prepare DMA
from an atomic context?

> +}
> +
> +static void d40_desc_lcla_runtime_put(struct d40_chan *d40c,

[ ... ]

> @@ -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);

[Severity: Medium]
Could this unnecessarily activate the LCLA SRAM power domain for physical
channels?

In drivers/dma/ste_dma40.c, d40_prep_sg() unconditionally resumes the
domain here before checking if the channel is logical or physical. Since
physical channels map directly to hardware registers and do not use the
LCLA SRAM, does this unnecessarily keep the power domain active and
introduce wakeup latency for physical channels?

> +	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);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921-ux500-power-domains-v7-1-v3-0-9352be6b86b1@kernel.org?part=15