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

[Marek Vasut <marex@denx.de>]

On 7/19/21 11:01 AM, Charles Keepax wrote:
> On Fri, Jul 16, 2021 at 08:21:33PM +0200, Marek Vasut wrote:
>> The original implementation of RPM handling in probe() was mostly
>> correct, except it failed to call pm_runtime_get_*() to activate the
>> hardware. The subsequent fix, 734882a8bf98 ("spi: cadence: Correct
>> initialisation of runtime PM"), breaks the implementation further,
>> to the point where the system using this hard IP on ZynqMP hangs on
>> boot, because it accesses hardware which is gated off.
>>
>> Undo 734882a8bf98 ("spi: cadence: Correct initialisation of runtime
>> PM") and instead add missing pm_runtime_get_noresume() and move the
>> RPM disabling all the way to the end of probe(). That makes ZynqMP
>> not hang on boot yet again.
>>
>> Fixes: 734882a8bf98 ("spi: cadence: Correct initialisation of runtime PM")
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Charles Keepax <ckeepax@opensource.cirrus.com>
>> Cc: Mark Brown <broonie@kernel.org>
>> ---
> 
> For my own edification do you know exactly what the problem was
> on your system here? I am assuming my mistake was that without the
> pm_runtime reference being taken, some required parent doesn't get
> enabled, which is convienently fine on my Zynq but not your ZynqMP?

Yes, the hardware got suspended a bit too early.

> The inclusion of the IRQ stuff in the pm_runtime block makes me a
> little nervous as if the problem is that your hardware generates
> a spurious IRQ on boot and that is where the bad access comes from
> this code feels racy. The original code did the put before the IRQ
> registers as well.

You could get an interrupt indeed, but that's not the case here.

> All that said, works on my Zynq:
> 
> Tested-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks !