Re: [PATCH] spi: tegra20-slink: Ensure SPI controller reset is deasserted

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2146 bytes --]

On Tue, Jun 08, 2021 at 08:15:18AM +0100, Jon Hunter wrote:
> Commit 4782c0a5dd88 ("clk: tegra: Don't deassert reset on enabling
> clocks") removed some legacy code for handling resets on Tegra from
> within the Tegra clock code. This exposed an issue in the Tegra20 slink
> driver where the SPI controller reset was not being deasserted as needed
> during probe. This is causing the Tegra30 Cardhu platform to hang on
> boot. Fix this by ensuring the SPI controller reset is deasserted during
> probe.
> 
> Fixes: 4782c0a5dd88 ("clk: tegra: Don't deassert reset on enabling clocks")

While it technically fixes an issue uncovered by that patch, I would
argue that the underlying issue has been present forever. So I think
this should be applied regardless of the above patch.

It also makes me wonder if we shouldn't drop the clock patch for now to
unbreak things and avoid having to model complicated dependencies to
make sure everything continues to work in v5.14-rc1.

Unless perhaps if Mark applies this for v5.13, then we can merge the
clock patch for v5.14-rc1 since SPI is the only IP that seems to be
broken by that change.

> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  drivers/spi/spi-tegra20-slink.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/spi/spi-tegra20-slink.c b/drivers/spi/spi-tegra20-slink.c
> index f7c832fd4003..6a726c95ac7a 100644
> --- a/drivers/spi/spi-tegra20-slink.c
> +++ b/drivers/spi/spi-tegra20-slink.c
> @@ -1118,6 +1118,11 @@ static int tegra_slink_probe(struct platform_device *pdev)
>  		pm_runtime_put_noidle(&pdev->dev);
>  		goto exit_pm_disable;
>  	}
> +
> +	reset_control_assert(tspi->rst);
> +	udelay(2);
> +	reset_control_deassert(tspi->rst);
> +

I wonder if this doesn't break now again on suspend/resume. Should we
perhaps move this into tegra_slink_runtime_resume()? Or better yet, move
the reset_control_assert() into tegra_slink_runtime_suspend() and the
reset_control_deassert() into tegra_slink_runtime_resume(). That should
ensure the device's reset is always deasserted when runtime resumed.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]