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

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

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

On Mon, Apr 27, 2020 at 04:15:21PM +0300, Dmitry Osipenko wrote:
> 27.04.2020 13:38, Wolfram Sang пишет:
> > On Mon, Apr 27, 2020 at 12:52:10PM +0300, Dmitry Osipenko wrote:
> >> 27.04.2020 10:48, Thierry Reding пишет:
> >> ...
> >>>> 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.
> >>
> >> There is a plenty of time to work on the proper fix now. To me it sounds
> >> like you're giving up on fixing the root of the problem, sorry.
> > 
> > From what I understood, there were (at least) two regressions reported.
> > So, to me, it makes sense to revert the change, so for upstream users
> > everything stays "the same". Of course, this does not mean it should
> > stay like this forever and you guys can work on fixing the root causes.
> > I'll happily apply them for this release when you are confident with the
> > results.
> > 
> 
> For now it's a single regression in the PCIe driver and it's actually
> not a regression, but a PCIe driver bug that needs to be fixed. The I2C
> part should be okay.
> 
> By reverting the I2C patch, we're back to the PCIe bug being papered
> over and I don't like this. Let's just fix the PCIe driver and the
> problem is gone.. it needs to be fixed anyways.

Yes, that bug should be fixed anyway. But that doesn't justify breaking
suspend/resume completely, which *is* a regression.

Look, I'm not saying that we should drop this patch altogether. All I'm
saying is that we should postpone it so that we can: a) get suspend and
resume working again (and by doing so make sure no other suspend/resume
regressions silently creep in, because that always seems to happen when
you're not looking) and b) fix any preexisting issues without possibly
scrambling the result with this perhaps unrelated fix.

So, again, I think the safest road forward is to back this one out for
now, fix whatever this other bug is and once suspend/resume is working
properly again we can revisit this patch based on a known-good baseline.

Thierry

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