08.06.2021 16:16, Thierry Reding пишет: >>> 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. >> Yes that works too. Will be great if this SPI fix could be merged into 5.13. It took two kernel releases to fix the audio resets, so I prefer not to postpone the clk patch again. >>>> Signed-off-by: Jon Hunter <jonathanh@xxxxxxxxxx> >>>> --- >>>> 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. >> So we do test suspend/resume on Cardhu and I have seen no issues with >> this applied. At first I did put this in the runtime_suspend/resume >> handlers, but then looking at what is done in spi-tegra114.c it appears >> we just do this on probe. See ... >> >> commit 019194933339b3e9b486639c8cb3692020844d65 >> Author: Sowjanya Komatineni <skomatineni@xxxxxxxxxx> >> Date: Tue Mar 26 22:56:32 2019 -0700 >> >> spi: tegra114: reset controller on probe >> >> I guess moving it to the runtime_suspend/resume handlers would be more >> consistent with the previous code. What do you think? > Do we test that SPI is still functional after suspend/resume? If it is, > I have no objection to this patch. I think making this part of runtime > suspend/resume would be a bit more correct or robust, but it's also a > bit more complicated and might introduce other problems. For example, > I suspect that if we reset on runtime suspend/resume, we would likely > need to reprogram the SLINK_COMMAND* registers as well. We don't support LP0 suspend state for older Tegra SoCs where hardware state is lost after suspending. Lot's of device drivers don't do suspend/resume properly. Fixing suspend/resume should be a separate patch, IMO.
