RE: [PATCH v4 14/17] dmaengine: sh: rz-dmac: Add suspend to RAM support

[Biju Das]

Hi Claudiu,

> -----Original Message-----
> From: Claudiu Beznea <claudiu.beznea@xxxxxxxxx>
> Sent: 20 April 2026 15:15
> Subject: Re: [PATCH v4 14/17] dmaengine: sh: rz-dmac: Add suspend to RAM support
> 
> 
> 
> On 4/20/26 10:42, Biju Das wrote:
> >> +static int rz_dmac_suspend(struct device *dev) {
> >> +	struct rz_dmac *dmac = dev_get_drvdata(dev);
> >> +	int ret;
> >> +
> >> +	for (unsigned int i = 0; i < dmac->n_channels; i++) {
> >> +		struct rz_dmac_chan *channel = &dmac->channels[i];
> >> +
> >> +		guard(spinlock_irqsave)(&channel->vc.lock);
> >> +
> >> +		if (!(channel->status & BIT(RZ_DMAC_CHAN_STATUS_CYCLIC)))
> >> +			continue;
> >> +
> >> +		ret = rz_dmac_device_pause_internal(channel);
> >> +		if (ret) {
> >> +			dev_err(dev, "Failed to suspend channel %s\n",
> >> +				dma_chan_name(&channel->vc.chan));
> >> +			break;
> >> +		}
> >> +
> >> +		channel->pm_state.nxla = rz_dmac_ch_readl(channel, NXLA, 1);
> >> +	}
> >> +
> >> +	if (ret) {
> >> +		rz_dmac_suspend_recover(dmac);
> >> +		return ret;
> >> +	}
> >> +
> >> +	pm_runtime_put_sync(dmac->dev);
> >> +
> >> +	ret = reset_control_assert(dmac->rstc);
> >> +	if (ret) {
> >> +		pm_runtime_resume_and_get(dmac->dev);
> >> +		rz_dmac_suspend_recover(dmac);
> >> +	}
> >> +
> >> +	return ret;
> >> +}
> >> +
> >> +static int rz_dmac_resume(struct device *dev) {
> >> +	struct rz_dmac *dmac = dev_get_drvdata(dev);
> >> +	int errors = 0, ret;
> >> +
> >> +	ret = reset_control_deassert(dmac->rstc);
> >> +	if (ret)
> >> +		return ret;
> >> +
> >> +	ret = pm_runtime_resume_and_get(dmac->dev);
> >
> > If this fails for any reason, the next suspend still be called and it will decrement the counter,
> potentially undeflowing it.
> > Consider switching to pm_runtime_get_sync(), which suits better here
> 
> 
> I think runtime PM usage counter underflow will be the less significant problem in case runtime PM
> fails.
> 
> Anyhow, could you please provide the code pattern you consider would be better for both suspend and
> resume?


system_resume()
{
          pm_runtime_resume_and_get() --> PM counter is not incremented in case of error
}
 
system_suspend()
{
       pm_runtime_put() --> counter is decremented and prints a noisy WARN message
}

Just replace pm_runtime_resume_and_get()->pm_runtime_get_sync() 
this will return the error to caller like previously and also increment the counter
which avoids warning on the subsequent suspend()


Cheers,
Biju