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
>
