Re: [PATCH] ASoC: da7219: Fix pole orientation detection on OMTP headsets when playing music

[Guenter Roeck]

On 2/8/23 19:05, David Rau wrote:
> -----Original Message-----
> From: Guenter Roeck <groeck7@xxxxxxxxx> On Behalf Of Guenter Roeck
> Sent: Thursday, February 9, 2023 02:04
> To: David Rau <david.rau.zg@xxxxxxxxxxx>
> Cc: Mark Brown <broonie@xxxxxxxxxx>; perex@xxxxxxxx; lgirdwood@xxxxxxxxx; tiwai@xxxxxxxx; support.opensource@xxxxxxxxxxx; alsa-devel@xxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] ASoC: da7219: Fix pole orientation detection on OMTP headsets when playing music
>
> On Tue, Feb 07, 2023 at 02:42:14AM +0000, David Rau wrote:
> > > > You mean just keep the above two patches and revert 969357ec94e6 ?
> > > > Sure, I can do that, but feedback from the field would take some
> > > > 2-3 months. Is that what you recommend to do for now ?
> > >
> > > > Thanks,
> > > > Guenter
> > >
> > > Thanks for the feedback.
> > > What I mean is just do rollback to remove the "sleep" patch I did in your repository.
> > >
> > > After the rollback, could you please let me know the average number of crashes per affected system with the same test conditions?
> > > Will it still take some 2-3 months?
> > The msleep() patch has been reverted in R111 and dev releases of ChromeOS. I did not get permission to land the revert in R110, meaning we'll continue to see the crashes there.
> > R111 is expected to go to Beta shortly, so we should get _some_ feedback in the next few weeks.
> > Still, it would be great to get a more permanent fix for the underlying problem. Also, the msleep() patch is still upstream, so a solution is still needed there.
> > I can try to get and play with one of the affected Chromebooks, but I don't think that would help much since we still don't know what the undocumented ground switch is supposed to do.
> Enable the GND switch earlier is needed to ensure the stable and smooth Jack detection.

"earlier" doesn't explain the changes, nor what "earlier" actually means.
The original code enabled the ground switch in the probe function,
only it never re-enabled it after the first insertion/removal sequence.
I don't know the potential impact on power consumption, but an easier
and more straightforward fix might be to (re-)enable the ground switch
on jack removal and to keep it enabled while nothing is inserted
(assuming that helps with detection).

The new (current) code enables the ground switch with each interrupt,
even if that interrupt is a button interrupt. That is really difficult
to understand and doesn't seem to make sense.

A diagram such as the one in Figure 20, describing exactly when the
ground switch is supposed to be enabled, would help a lot. It would
be even better if that diagram provided information about any extra
delays needed after e_jack_detect_complete is raised and before the
detection code actually runs.

> > Thanks,
> > Guenter
>
> Thanks for the kind explanation and feedback.
> I am verifying another method which do the msleep() in the individual schedule work to avoid such crash issue.
>
> Would you please let me know how to reproduce this crash phenomenon?
> Then I can ensure the new solution is stronger and solve this problem as well.

Sorry, I can't, because the crash reports are from customers and automated.
We don't know what they are doing, only that whatever they are doing
causes the hang and thus crash.

Thanks,
Guenter