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

[Claudiu Beznea <claudiu.beznea@tuxon.dev>]

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?

Thank you,
Claudiu