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

[David Rau]

Would you please provide me the related error messages when hung task crashes in da7219_aad_irq_thread()?
BTW, "gnd_switch_delay = 256" is an unusual use case of the longer jack detection latency. 

-----Original Message-----
From: Guenter Roeck <groeck7@xxxxxxxxx> On Behalf Of Guenter Roeck
Sent: Wednesday, January 18, 2023 03:57
To: David Rau <we730128@xxxxxxxxx>
Cc: perex@xxxxxxxx; lgirdwood@xxxxxxxxx; broonie@xxxxxxxxxx; tiwai@xxxxxxxx; support.opensource@xxxxxxxxxxx; alsa-devel@xxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; David Rau <david.rau.zg@xxxxxxxxxxx>
Subject: Re: [PATCH] ASoC: da7219: Fix pole orientation detection on OMTP headsets when playing music

On Mon, Nov 21, 2022 at 05:07:44AM +0000, David Rau wrote:
> The OMTP pin define headsets can be mis-detected as line out instead 
> of OMTP, causing obvious issues with audio quality.
> This patch is to put increased resistances within the device at a 
> suitable point.
> 
> To solve this issue better, the new mechanism setup ground switches 
> with conditional delay control and these allow for more stabile 
> detection process to operate as intended. This conditional delay 
> control will not impact the hardware process but use extra system 
> resource.
> 
> This commit improves control of ground switches in the AAD logic.
> 
> Signed-off-by: David Rau <david.rau.zg@xxxxxxxxxxx>
> ---
>  sound/soc/codecs/da7219-aad.c | 42 
> ++++++++++++++++++++++++++++++-----
>  sound/soc/codecs/da7219-aad.h |  1 +
>  2 files changed, 37 insertions(+), 6 deletions(-)
> 
> diff --git a/sound/soc/codecs/da7219-aad.c 
> b/sound/soc/codecs/da7219-aad.c index bba73c44c219..08200ec259f9 
> 100644
> --- a/sound/soc/codecs/da7219-aad.c
> +++ b/sound/soc/codecs/da7219-aad.c
> @@ -352,9 +352,14 @@ static irqreturn_t da7219_aad_irq_thread(int irq, void *data)
>  	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
>  	struct da7219_priv *da7219 = snd_soc_component_get_drvdata(component);
>  	u8 events[DA7219_AAD_IRQ_REG_MAX];
> -	u8 statusa;
> +	u8 statusa, srm_st;
>  	int i, report = 0, mask = 0;
>  
> +	srm_st = snd_soc_component_read(component, DA7219_PLL_SRM_STS) & DA7219_PLL_SRM_STS_MCLK;
> +	msleep(da7219_aad->gnd_switch_delay * ((srm_st == 0x0) ? 2 : 1) - 
> +4);

Ever since this patch was applied to ChromeOS, we have observed hung task crashes in da7219_aad_irq_thread().

Is it really appropriate to sleep up to (256 * 2) - 4 = 508 ms in an interrupt handler ?

Thanks,
Guenter

> +	/* Enable ground switch */
> +	snd_soc_component_update_bits(component, 0xFB, 0x01, 0x01);
> +
>  	/* Read current IRQ events */
>  	regmap_bulk_read(da7219->regmap, DA7219_ACCDET_IRQ_EVENT_A,
>  			 events, DA7219_AAD_IRQ_REG_MAX);
> @@ -454,8 +459,8 @@ static irqreturn_t da7219_aad_irq_thread(int irq, void *data)
>  			snd_soc_dapm_disable_pin(dapm, "Mic Bias");
>  			snd_soc_dapm_sync(dapm);
>  
> -			/* Enable ground switch */
> -			snd_soc_component_update_bits(component, 0xFB, 0x01, 0x01);
> +			/* Disable ground switch */
> +			snd_soc_component_update_bits(component, 0xFB, 0x01, 0x00);
>  		}
>  	}
>  
> @@ -831,6 +836,32 @@ static void da7219_aad_handle_pdata(struct snd_soc_component *component)
>  	}
>  }
>  
> +static void da7219_aad_handle_gnd_switch_time(struct 
> +snd_soc_component *component) {
> +	struct da7219_priv *da7219 = snd_soc_component_get_drvdata(component);
> +	struct da7219_aad_priv *da7219_aad = da7219->aad;
> +	u8 jack_det;
> +
> +	jack_det = snd_soc_component_read(component, DA7219_ACCDET_CONFIG_2)
> +		& DA7219_JACK_DETECT_RATE_MASK;
> +	switch (jack_det) {
> +	case 0x00:
> +		da7219_aad->gnd_switch_delay = 32;
> +		break;
> +	case 0x10:
> +		da7219_aad->gnd_switch_delay = 64;
> +		break;
> +	case 0x20:
> +		da7219_aad->gnd_switch_delay = 128;
> +		break;
> +	case 0x30:
> +		da7219_aad->gnd_switch_delay = 256;
> +		break;
> +	default:
> +		da7219_aad->gnd_switch_delay = 32;
> +		break;
> +	}
> +}
>  
>  /*
>   * Suspend/Resume
> @@ -908,9 +939,6 @@ int da7219_aad_init(struct snd_soc_component *component)
>  	snd_soc_component_update_bits(component, DA7219_ACCDET_CONFIG_1,
>  			    DA7219_BUTTON_CONFIG_MASK, 0);
>  
> -	/* Enable ground switch */
> -	snd_soc_component_update_bits(component, 0xFB, 0x01, 0x01);
> -
>  	INIT_WORK(&da7219_aad->btn_det_work, da7219_aad_btn_det_work);
>  	INIT_WORK(&da7219_aad->hptest_work, da7219_aad_hptest_work);
>  
> @@ -928,6 +956,8 @@ int da7219_aad_init(struct snd_soc_component *component)
>  	regmap_bulk_write(da7219->regmap, DA7219_ACCDET_IRQ_MASK_A,
>  			  &mask, DA7219_AAD_IRQ_REG_MAX);
>  
> +	da7219_aad_handle_gnd_switch_time(component);
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(da7219_aad_init);
> diff --git a/sound/soc/codecs/da7219-aad.h 
> b/sound/soc/codecs/da7219-aad.h index f48a12012ef3..21fdf53095cc 
> 100644
> --- a/sound/soc/codecs/da7219-aad.h
> +++ b/sound/soc/codecs/da7219-aad.h
> @@ -187,6 +187,7 @@ enum da7219_aad_event_regs {  struct 
> da7219_aad_priv {
>  	struct snd_soc_component *component;
>  	int irq;
> +	int gnd_switch_delay;
>  
>  	u8 micbias_pulse_lvl;
>  	u32 micbias_pulse_time;
> --
> 2.17.1
>