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

[Shubhrajyoti Datta <shubhrajyoti.datta@xxxxxxxxx>]

Hi Charles,

On Mon, Jan 7, 2019 at 2:52 PM Charles Keepax
<ckeepax@xxxxxxxxxxxxxxxxxxxxx> wrote:
>
> 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.
>


BTW  after the probe the clocks are left enabled ?

> Thanks,
> Charles