Re: [PATCH] spi: cadence: Correct initialisation of runtime PM

[Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxx>]

On Mon, Jan 07, 2019 at 11:26:36AM +0530, Shubhrajyoti Datta wrote:
> Hi Charles,
> 
> On Fri, Jan 4, 2019 at 11:40 PM Charles Keepax
> <ckeepax@xxxxxxxxxxxxxxxxxxxxx> wrote:
> >
> > Currently the driver calls pm_runtime_put_autosuspend but without ever
> > having done a pm_runtime_get, this causes the reference count in the pm
> 
> Before every transfer there is a prepare and then the get_sync  gets
> called as we
> have the auto_runtime_pm as true.
> 
> let me know if I am missing something?

When the code makes the call to:

pm_runtime_set_active(&pdev->dev);

That informs the PM runtime core that the device is currently
active, however it does not increment the PM runtimes reference
count. So the reference count is still 0 and the device will not
be prevented from suspending. The driver requires the device to
be powered up for the call to:

cdns_spi_init_hw(xspi);

Currently there is nothing that prevents it from suspending
before then, other than it normally running through the code
before SPI_AUTOSUSPEND_TIMEOUT expires. Additionally though the
code then calls:

pm_runtime_mark_last_busy(&pdev->dev);
pm_runtime_put_autosuspend(&pdev->dev);

Which will decrease the PM reference count to -1. This is very
bad as the first PM runtime get will only increase it to zero, which
still allows the device to suspend. It is an unbalanced put and
the net effect is it cancels out the first get.

Now what appears to happen on my system is that historically
there has always been enough lag in the system that a second PM
runtime get happens before the device actually suspends, allowing
things to still work. However this patch to update the timers in
the PM runtime core:

8234f6734c5d ("PM-runtime: Switch autosuspend over to using hrtimers")

Tightened up the response time and now sometimes I get the device
suspending before a transaction is full complete, causing
failures.

Thanks,
Charles