On 02-12-24, 19:51, Stefan Wahren wrote: > Hi Vinod, > > Am 02.12.24 um 17:52 schrieb Vinod Koul: > > On 25-10-24, 12:36, Stefan Wahren wrote: > > > bcm2835-dma provides the service to others, so it should > > > suspend late and resume early. > > > > > > Signed-off-by: Stefan Wahren <wahrenst@xxxxxxx> > > > --- > > > drivers/dma/bcm2835-dma.c | 30 ++++++++++++++++++++++++++++++ > > > 1 file changed, 30 insertions(+) > > > > > > diff --git a/drivers/dma/bcm2835-dma.c b/drivers/dma/bcm2835-dma.c > > > index e1b92b4d7b05..647dda9f3376 100644 > > > --- a/drivers/dma/bcm2835-dma.c > > > +++ b/drivers/dma/bcm2835-dma.c > > > @@ -875,6 +875,35 @@ static struct dma_chan *bcm2835_dma_xlate(struct of_phandle_args *spec, > > > return chan; > > > } > > > > > > +static int bcm2835_dma_suspend_late(struct device *dev) > > > +{ > > > + struct bcm2835_dmadev *od = dev_get_drvdata(dev); > > > + struct bcm2835_chan *c, *next; > > > + > > > + list_for_each_entry_safe(c, next, &od->ddev.channels, > > > + vc.chan.device_node) { > > > + void __iomem *chan_base = c->chan_base; > > > + > > > + if (readl(chan_base + BCM2835_DMA_ADDR)) { > > > + dev_warn(dev, "Suspend is prevented by chan %d\n", > > > + c->ch); > > > + return -EBUSY; > > > + } > > Can you help understand how this helps by logging... we are not adding > > anything except checking this and resume is NOP as well! > My intention of this patch is just to make sure, that no DMA transfer is > in progress during late_suspend. So i followed the implementation of > fsldma.c > > Additionally i added this warning mostly to know if this ever occurs. > But i wasn't able to trigger. Okay in the case I dont think it is a abd idea. But the patch description should mention that add a warning while suspending if channels are active or something like that. Patch title and description should mention this.. > > Should i drop the warning and make resume callback = NULL? Yes that would be better as well, no point in having dummy code -- ~Vinod