[PATCH 0/2] ASoC: da7219: Patches related to a spurious AAD IRQ issue

[PATCH 0/2] ASoC: da7219: Patches related to a spurious AAD IRQ issue

[Dmytro Maluka]

This series includes 2 patches related to (but not fixing) the following
I2C failure which occurs sometimes during system suspend or resume and
indicates a problem with a spurious DA7219 interrupt:

[  355.876211] i2c_designware i2c_designware.3: Transfer while suspended
[  355.876245] WARNING: CPU: 2 PID: 3576 at drivers/i2c/busses/i2c-designware-master.c:570 i2c_dw_xfer+0x411/0x440
...
[  355.876462] Call Trace:
[  355.876468]  <TASK>
[  355.876475]  ? update_load_avg+0x1b3/0x615
[  355.876484]  __i2c_transfer+0x101/0x1d8
[  355.876494]  i2c_transfer+0x74/0x10d
[  355.876504]  regmap_i2c_read+0x6a/0x9c
[  355.876513]  _regmap_raw_read+0x179/0x223
[  355.876521]  regmap_raw_read+0x1e1/0x28e
[  355.876527]  regmap_bulk_read+0x17d/0x1ba
[  355.876532]  ? __wake_up+0xed/0x1bb
[  355.876542]  da7219_aad_irq_thread+0x54/0x2c9 [snd_soc_da7219 5fb8ebb2179cf2fea29af090f3145d68ed8e2184]
[  355.876556]  irq_thread+0x13c/0x231
[  355.876563]  ? irq_forced_thread_fn+0x5f/0x5f
[  355.876570]  ? irq_thread_fn+0x4d/0x4d
[  355.876576]  kthread+0x13a/0x152
[  355.876581]  ? synchronize_irq+0xc3/0xc3
[  355.876587]  ? kthread_blkcg+0x31/0x31
[  355.876592]  ret_from_fork+0x1f/0x30
[  355.876601]  </TASK>

This log shows that DA7219 AAD interrupt handler da7219_aad_irq_thread()
is unexpectedly running when DA7219 is suspended and should not generate
interrupts. As a result, the IRQ handler is trying to read AAD IRQ event
status over I2C and is hitting the I2C driver "Transfer while suspended"
failure.

Patch #1 adds synchronize_irq() when suspending DA7219, to prevent the
IRQ handler from running after suspending if there is a pending IRQ
generated before suspending. With this patch the above failure is still
reproducible, so this patch does not fix any real observed issue so far,
but at least is useful for confirming that the above issue is not caused
by a pending IRQ but rather looks like a DA7219 hardware issue with an
unexpectedly generated IRQ.

Patch #2 does not fix the above issue either, but it prevents its
potentially harmful side effects. With the existing code, if the issue
occurs and the IRQ handler fails to read the AAD IRQ events status over
I2C, it does not check that and tries to use the garbage uninitialized
value of the events status, potentially reporting bogus events. This
patch fixes that by adding missing error checking.

In fact I'm sending these patches not only to submit them for review but
also to ask Renesas folks for any hints on a possible cause of the
described DA7219 issue (AAD interrupts spuriously firing after jack
detection is already disabled) or how to debug it further.

Dmytro Maluka (2):
  ASoC: da7219: Flush pending AAD IRQ when suspending
  ASoC: da7219: Check for failure reading AAD IRQ events

 sound/soc/codecs/da7219-aad.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)


base-commit: fdf0eaf11452d72945af31804e2a1048ee1b574c
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH 1/2] ASoC: da7219: Flush pending AAD IRQ when suspending

[Dmytro Maluka]

da7219_aad_suspend() disables jack detection, which should prevent
generating new interrupts by DA7219 while suspended. However, there is a
theoretical possibility that there is a pending interrupt generated just
before suspending DA7219 and not handled yet, so the IRQ handler may
still run after DA7219 is suspended. To prevent that, wait until the
pending IRQ handling is done.

This patch arose as an attempt to fix the following I2C failure
occurring sometimes during system suspend or resume:

[  355.876211] i2c_designware i2c_designware.3: Transfer while suspended
[  355.876245] WARNING: CPU: 2 PID: 3576 at drivers/i2c/busses/i2c-designware-master.c:570 i2c_dw_xfer+0x411/0x440
...
[  355.876462] Call Trace:
[  355.876468]  <TASK>
[  355.876475]  ? update_load_avg+0x1b3/0x615
[  355.876484]  __i2c_transfer+0x101/0x1d8
[  355.876494]  i2c_transfer+0x74/0x10d
[  355.876504]  regmap_i2c_read+0x6a/0x9c
[  355.876513]  _regmap_raw_read+0x179/0x223
[  355.876521]  regmap_raw_read+0x1e1/0x28e
[  355.876527]  regmap_bulk_read+0x17d/0x1ba
[  355.876532]  ? __wake_up+0xed/0x1bb
[  355.876542]  da7219_aad_irq_thread+0x54/0x2c9 [snd_soc_da7219 5fb8ebb2179cf2fea29af090f3145d68ed8e2184]
[  355.876556]  irq_thread+0x13c/0x231
[  355.876563]  ? irq_forced_thread_fn+0x5f/0x5f
[  355.876570]  ? irq_thread_fn+0x4d/0x4d
[  355.876576]  kthread+0x13a/0x152
[  355.876581]  ? synchronize_irq+0xc3/0xc3
[  355.876587]  ? kthread_blkcg+0x31/0x31
[  355.876592]  ret_from_fork+0x1f/0x30
[  355.876601]  </TASK>

which indicates that the AAD IRQ handler is unexpectedly running when
DA7219 is suspended, and as a result, is trying to read data from DA7219
over I2C and is hitting the I2C driver "Transfer while suspended"
failure.

However, with this patch the above failure is still reproducible. So
this patch does not fix any real observed issue so far, but at least is
useful for confirming that the above issue is not caused by a pending
IRQ but rather looks like a DA7219 hardware issue with an IRQ
unexpectedly generated after jack detection is already disabled.

Signed-off-by: Dmytro Maluka <dmy@semihalf.com>
---
 sound/soc/codecs/da7219-aad.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sound/soc/codecs/da7219-aad.c b/sound/soc/codecs/da7219-aad.c
index c65256bd526d..202715b7bbea 100644
--- a/sound/soc/codecs/da7219-aad.c
+++ b/sound/soc/codecs/da7219-aad.c
@@ -944,6 +944,8 @@ void da7219_aad_suspend(struct snd_soc_component *component)
 			}
 		}
 	}
+
+	synchronize_irq(da7219_aad->irq);
 }
 
 void da7219_aad_resume(struct snd_soc_component *component)
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH 2/2] ASoC: da7219: Check for failure reading AAD IRQ events

[Dmytro Maluka]

When handling an AAD interrupt, if IRQ events read failed (for example,
due to i2c "Transfer while suspended" failure, i.e. when attempting to
read it while DA7219 is suspended, which may happen due to a spurious
AAD interrupt), the events array contains garbage uninitialized values.
So instead of trying to interprete those values and doing any actions
based on them (potentially resulting in misbehavior, e.g. reporting
bogus events), refuse to handle the interrupt.

Signed-off-by: Dmytro Maluka <dmy@semihalf.com>
---
 sound/soc/codecs/da7219-aad.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/da7219-aad.c b/sound/soc/codecs/da7219-aad.c
index 202715b7bbea..581b334a6631 100644
--- a/sound/soc/codecs/da7219-aad.c
+++ b/sound/soc/codecs/da7219-aad.c
@@ -361,11 +361,15 @@ static irqreturn_t da7219_aad_irq_thread(int irq, void *data)
 	struct da7219_priv *da7219 = snd_soc_component_get_drvdata(component);
 	u8 events[DA7219_AAD_IRQ_REG_MAX];
 	u8 statusa;
-	int i, report = 0, mask = 0;
+	int i, ret, report = 0, mask = 0;
 
 	/* Read current IRQ events */
-	regmap_bulk_read(da7219->regmap, DA7219_ACCDET_IRQ_EVENT_A,
-			 events, DA7219_AAD_IRQ_REG_MAX);
+	ret = regmap_bulk_read(da7219->regmap, DA7219_ACCDET_IRQ_EVENT_A,
+			       events, DA7219_AAD_IRQ_REG_MAX);
+	if (ret) {
+		dev_warn_ratelimited(component->dev, "Failed to read IRQ events: %d\n", ret);
+		return IRQ_NONE;
+	}
 
 	if (!events[DA7219_AAD_IRQ_REG_A] && !events[DA7219_AAD_IRQ_REG_B])
 		return IRQ_NONE;
-- 
2.41.0.255.g8b1d071c50-goog

Re: [PATCH 0/2] ASoC: da7219: Patches related to a spurious AAD IRQ issue

[Mark Brown]

On Mon, 17 Jul 2023 21:37:35 +0200, Dmytro Maluka wrote:
> This series includes 2 patches related to (but not fixing) the following
> I2C failure which occurs sometimes during system suspend or resume and
> indicates a problem with a spurious DA7219 interrupt:
> 
> [  355.876211] i2c_designware i2c_designware.3: Transfer while suspended
> [  355.876245] WARNING: CPU: 2 PID: 3576 at drivers/i2c/busses/i2c-designware-master.c:570 i2c_dw_xfer+0x411/0x440
> ...
> [  355.876462] Call Trace:
> [  355.876468]  <TASK>
> [  355.876475]  ? update_load_avg+0x1b3/0x615
> [  355.876484]  __i2c_transfer+0x101/0x1d8
> [  355.876494]  i2c_transfer+0x74/0x10d
> [  355.876504]  regmap_i2c_read+0x6a/0x9c
> [  355.876513]  _regmap_raw_read+0x179/0x223
> [  355.876521]  regmap_raw_read+0x1e1/0x28e
> [  355.876527]  regmap_bulk_read+0x17d/0x1ba
> [  355.876532]  ? __wake_up+0xed/0x1bb
> [  355.876542]  da7219_aad_irq_thread+0x54/0x2c9 [snd_soc_da7219 5fb8ebb2179cf2fea29af090f3145d68ed8e2184]
> [  355.876556]  irq_thread+0x13c/0x231
> [  355.876563]  ? irq_forced_thread_fn+0x5f/0x5f
> [  355.876570]  ? irq_thread_fn+0x4d/0x4d
> [  355.876576]  kthread+0x13a/0x152
> [  355.876581]  ? synchronize_irq+0xc3/0xc3
> [  355.876587]  ? kthread_blkcg+0x31/0x31
> [  355.876592]  ret_from_fork+0x1f/0x30
> [  355.876601]  </TASK>
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/2] ASoC: da7219: Flush pending AAD IRQ when suspending
      commit: 91e292917dad64ab8d1d5ca2ab3069ad9dac6f72
[2/2] ASoC: da7219: Check for failure reading AAD IRQ events
      commit: f0691dc16206f21b13c464434366e2cd632b8ed7

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark