linux-kernel - Re: [PATCH v2 1/2] i2c: tegra: Better handle case where CPU0 is busy for a long time

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200427074837.GC3451400@ulmo>
Date:   Mon, 27 Apr 2020 09:48:37 +0200
From:   Thierry Reding <thierry.reding@...il.com>
To:     Wolfram Sang <wsa@...-dreams.de>
Cc:     Jon Hunter <jonathanh@...dia.com>,
        Dmitry Osipenko <digetx@...il.com>,
        Laxman Dewangan <ldewangan@...dia.com>,
        Manikanta Maddireddy <mmaddireddy@...dia.com>,
        Vidya Sagar <vidyas@...dia.com>, linux-i2c@...r.kernel.org,
        linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] i2c: tegra: Better handle case where CPU0 is busy
 for a long time

On Fri, Apr 24, 2020 at 04:19:25PM +0100, Jon Hunter wrote:
> 
> On 24/04/2020 15:45, Dmitry Osipenko wrote:
> > 24.04.2020 10:10, Jon Hunter пишет:
> > ...
> >>> Could you please clarify why pm_runtime_get_sync() can't be used by the
> >>> I2C driver's in NOIRQ phase?
> >>
> >> Yes take a look at commit 1e2ef05bb8cf ("PM: Limit race conditions
> >> between runtime PM and system sleep (v2)").
> > 
> > I2C driver now uses irq-safe RPM since ede2299f7 ("i2c: tegra: Support
> > atomic transfers"), and thus, the RPM's workqueue shouldn't be a
> > problem. I guess RPM should work fine in this case, don't you think so?
> 
> I was testing, and I did not see it using atomic transfers. I can
> confirm if the RPM callbacks are called or not, but I did not think so.
> However, let me confirm.
> 
> >>> Yes, keeping PCI regulators always-enabled should be a good immediate
> >>> solution.
> >>
> >> I was thinking about that, and I am not sure it is. I don't think that
> >> the failure to send the I2C command should break suspend.
> > 
> > It shouldn't, but looks like it should be a separate problem.
> 
> Maybe but all these other problems appear to have existed for sometime
> now. We need to fix all, but for the moment we need to figure out what's
> best for v5.7.

To me it doesn't sound like we have a good handle on what exactly is
going on here and we're mostly just poking around.

And even if things weren't working quite properly before, it sounds to
me like this patch actually made things worse.

Given all that, I think the best course of action at this point is to
revert for v5.7 and prevent this from spreading[0]. After that we need
to look at fixing the regulator issues and make sure that suspend/resume
actually works properly and without errors. After that we should have a
better chance of isolating why exactly this patch fails.

Wolfram, can you revert the following two patches for v5.7, please?

	8814044fe0fa i2c: tegra: Synchronize DMA before termination
	a900aeac2537 i2c: tegra: Better handle case where CPU0 is busy for a long time

Thanks,
Thierry

[0]: https://lkml.org/lkml/2020/4/24/498

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)