[PATCH] ALSA: hda: Reset pending verb count on response timeout

[PATCH] ALSA: hda: Reset pending verb count on response timeout

[songxiebing]

From: Bob Song <songxiebing@kylinos.cn>

On some controllers the HDA link reports a verb timeout and the driver
falls back to polling mode.  After that, every subsequent verb sent to
that codec address keeps timing out (roughly one second per verb), while
the hardware itself looks perfectly healthy: the RIRB write pointer keeps
advancing, interrupts are still delivered, and verbs that carry no reply
payload -- notably writes -- still take effect on the codec.

The reason is that snd_hdac_bus_send_cmd() increments
bus->rirb.cmds[addr] for every verb, but the counter is only decremented
by snd_hdac_bus_update_rirb() when a matching RIRB response is read, with
no rollback if the response never arrives.

When a controller cannot fall back to single-command mode, i.e.
chip->fallback_to_single_cmd is zero, azx_rirb_get_response() bails out
with -EIO immediately and never reaches the bus-reset / single_cmd
recovery that reinitializes CORB/RIRB and clears the counters (that path
is guarded by the same flag).  The ACPI platform, Tegra and CIX
controllers never set that flag, so they are all exposed to this.

A single lost response therefore poisons bus->rirb.cmds[addr]
permanently: each later verb still gets one response that only brings the
count back down to the stale baseline of 1, so
snd_hdac_bus_get_response() keeps timing out even though the codec
answers normally.

Fix this by dropping the pending count for the codec address once the
verb is known to be dead.  A late response is then handled as a spurious
response exactly as before, and the following verbs recover.

Signed-off-by: Bob Song <songxiebing@kylinos.cn>
---
 sound/hda/common/controller.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/sound/hda/common/controller.c b/sound/hda/common/controller.c
index afec5c5546ec..7a0b641fba14 100644
--- a/sound/hda/common/controller.c
+++ b/sound/hda/common/controller.c
@@ -773,7 +773,7 @@ static int azx_rirb_get_response(struct hdac_bus *bus, unsigned int addr,
 		return 0;
 
 	if (hbus->no_response_fallback)
-		return -EIO;
+		goto error;
 
 	if (!bus->polling_mode) {
 		dev_warn(chip->card->dev,
@@ -789,7 +789,7 @@ static int azx_rirb_get_response(struct hdac_bus *bus, unsigned int addr,
 			 bus->last_cmd[addr]);
 		if (chip->ops->disable_msi_reset_irq &&
 		    chip->ops->disable_msi_reset_irq(chip) < 0)
-			return -EIO;
+			goto error;
 		goto again;
 	}
 
@@ -798,12 +798,12 @@ static int azx_rirb_get_response(struct hdac_bus *bus, unsigned int addr,
 		 * phase, this is likely an access to a non-existing codec
 		 * slot.  Better to return an error and reset the system.
 		 */
-		return -EIO;
+		goto error;
 	}
 
 	/* no fallback mechanism? */
 	if (!chip->fallback_to_single_cmd)
-		return -EIO;
+		goto error;
 
 	/* a fatal communication error; need either to reset or to fallback
 	 * to the single_cmd mode
@@ -823,6 +823,18 @@ static int azx_rirb_get_response(struct hdac_bus *bus, unsigned int addr,
 	hbus->response_reset = 0;
 	snd_hdac_bus_stop_cmd_io(bus);
 	return -EIO;
+
+ error:
+	/*
+	 * The command will not get any response.  Drop its pending count,
+	 * otherwise bus->rirb.cmds[addr] stays non-zero forever and every
+	 * later verb to this codec address keeps timing out, even though its
+	 * own response is delivered normally.
+	 */
+	scoped_guard(spinlock_irq, &bus->reg_lock) {
+		bus->rirb.cmds[addr] = 0;
+	}
+	return -EIO;
 }
 
 /*
-- 
2.25.1

Re: [PATCH] ALSA: hda: Reset pending verb count on response timeout

[Takashi Iwai]

On Fri, 09 Oct 2026 05:40:40 +0200,
songxiebing wrote:
> 
> From: Bob Song <songxiebing@kylinos.cn>
> 
> On some controllers the HDA link reports a verb timeout and the driver
> falls back to polling mode.  After that, every subsequent verb sent to
> that codec address keeps timing out (roughly one second per verb), while
> the hardware itself looks perfectly healthy: the RIRB write pointer keeps
> advancing, interrupts are still delivered, and verbs that carry no reply
> payload -- notably writes -- still take effect on the codec.
> 
> The reason is that snd_hdac_bus_send_cmd() increments
> bus->rirb.cmds[addr] for every verb, but the counter is only decremented
> by snd_hdac_bus_update_rirb() when a matching RIRB response is read, with
> no rollback if the response never arrives.
> 
> When a controller cannot fall back to single-command mode, i.e.
> chip->fallback_to_single_cmd is zero, azx_rirb_get_response() bails out
> with -EIO immediately and never reaches the bus-reset / single_cmd
> recovery that reinitializes CORB/RIRB and clears the counters (that path
> is guarded by the same flag).  The ACPI platform, Tegra and CIX
> controllers never set that flag, so they are all exposed to this.
> 
> A single lost response therefore poisons bus->rirb.cmds[addr]
> permanently: each later verb still gets one response that only brings the
> count back down to the stale baseline of 1, so
> snd_hdac_bus_get_response() keeps timing out even though the codec
> answers normally.
> 
> Fix this by dropping the pending count for the codec address once the
> verb is known to be dead.  A late response is then handled as a spurious
> response exactly as before, and the following verbs recover.
> 
> Signed-off-by: Bob Song <songxiebing@kylinos.cn>

Thanks for the patch.

I think, however, that it's better to fix in the hda core side,
i.e. snd_hdac_bus_exec_verb_unlocked().  When bus->ops->get_response()
gets an error, just reset bus->rirb.cmds[addr].  Maybe create a helper
(e.g. snd_hdac_bus_reset_response_counter(buf, addr)) in
sound/hda/core/controller.c, and call it instead of open-code to make
the meaning clearer.

In this way, it'll cover all HD-audio controllers, not only hda-intel.


Takashi

[PATCH v2] ALSA: hda: Reset response counter when a verb times out

[songxiebing]

From: Bob Song <songxiebing@kylinos.cn>

On some controllers the HDA link reports a verb timeout and the driver
falls back to polling mode.  After that, every subsequent verb sent to
that codec address keeps timing out (roughly one second per verb), while
the hardware itself looks perfectly healthy: the RIRB write pointer keeps
advancing, interrupts are still delivered, and verbs that carry no reply
payload -- notably writes -- still take effect on the codec.

The reason is that bus->rirb.cmds[addr] is incremented for every verb and
only decremented in snd_hdac_bus_update_rirb() when a matching RIRB
response is read, without any rollback for a response that never arrives.
A controller that cannot fall back to the single-command mode
(chip->fallback_to_single_cmd == 0, e.g. the ACPI platform, Tegra and CIX
controllers) returns an error right away and never performs the bus reset
that would reinitialize CORB/RIRB and clear the counter, so a single lost
response leaves bus->rirb.cmds[addr] non-zero forever.  Each following
verb then still gets one response that only brings the count back to the
stale baseline of 1, and snd_hdac_bus_get_response() keeps timing out even
though the codec answers normally.

Handle it in the core helper snd_hdac_bus_exec_verb_unlocked(): whenever
get_response() reports an error, reset the pending counter for the codec
address, so a late response is simply treated as a spurious response and
the following verbs recover.  Doing it here covers all HD-audio
controllers instead of relying on each controller's fallback setting.

Add a small helper snd_hdac_bus_reset_response_counter() for this.

Signed-off-by: Bob Song <songxiebing@kylinos.cn>
---
Changes in v2:
- Move the fix from azx_rirb_get_response() in the HDA controller helper
  into the HD-audio core (snd_hdac_bus_exec_verb_unlocked()), as suggested
  by Takashi Iwai, so that it covers all HD-audio controllers and not only
  the ones using the shared azx helper.
- Add the snd_hdac_bus_reset_response_counter() helper instead of
  open-coding the counter reset.

 include/sound/hdaudio.h     |  2 ++
 sound/hda/core/bus.c        |  6 +++++-
 sound/hda/core/controller.c | 20 ++++++++++++++++++++
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index aa994d6e6d35..b0fbb61fa093 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -398,6 +398,8 @@ void snd_hdac_codec_link_down(struct hdac_device *codec);
 int snd_hdac_bus_send_cmd(struct hdac_bus *bus, unsigned int val);
 int snd_hdac_bus_get_response(struct hdac_bus *bus, unsigned int addr,
 			      unsigned int *res);
+void snd_hdac_bus_reset_response_counter(struct hdac_bus *bus,
+					 unsigned int addr);
 int snd_hdac_bus_parse_capabilities(struct hdac_bus *bus);
 
 bool snd_hdac_bus_init_chip(struct hdac_bus *bus, bool full_reset);
diff --git a/sound/hda/core/bus.c b/sound/hda/core/bus.c
index 81498f1e413e..ecc4b7a780a4 100644
--- a/sound/hda/core/bus.c
+++ b/sound/hda/core/bus.c
@@ -121,11 +121,15 @@ int snd_hdac_bus_exec_verb_unlocked(struct hdac_bus *bus, unsigned int addr,
 			break;
 		/* process pending verbs */
 		err = bus->ops->get_response(bus, addr, &tmp);
-		if (err)
+		if (err) {
+			snd_hdac_bus_reset_response_counter(bus, addr);
 			break;
+		}
 	}
 	if (!err && res) {
 		err = bus->ops->get_response(bus, addr, res);
+		if (err)
+			snd_hdac_bus_reset_response_counter(bus, addr);
 		trace_hda_get_response(bus, addr, *res);
 	}
 	return err;
diff --git a/sound/hda/core/controller.c b/sound/hda/core/controller.c
index 78855ac357c6..1babc5109049 100644
--- a/sound/hda/core/controller.c
+++ b/sound/hda/core/controller.c
@@ -396,6 +396,26 @@ int snd_hdac_bus_get_response(struct hdac_bus *bus, unsigned int addr,
 }
 EXPORT_SYMBOL_GPL(snd_hdac_bus_get_response);
 
+/**
+ * snd_hdac_bus_reset_response_counter - reset the pending response count
+ * @bus: HD-audio core bus
+ * @addr: codec address
+ *
+ * Drop the number of commands that are still waiting for a response from
+ * the given codec address.  This is called when a verb is known to be dead,
+ * e.g. after a response timeout, so that a lost response won't leave the
+ * counter non-zero forever and block all later verbs to the same codec.
+ */
+void snd_hdac_bus_reset_response_counter(struct hdac_bus *bus, unsigned int addr)
+{
+	if (addr >= HDA_MAX_CODECS)
+		return;
+
+	guard(spinlock_irq)(&bus->reg_lock);
+	bus->rirb.cmds[addr] = 0;
+}
+EXPORT_SYMBOL_GPL(snd_hdac_bus_reset_response_counter);
+
 #define HDAC_MAX_CAPS 10
 /**
  * snd_hdac_bus_parse_capabilities - parse capability structure
-- 
2.25.1

Re: [PATCH v2] ALSA: hda: Reset response counter when a verb times out

[Takashi Iwai]

On Sat, 10 Oct 2026 04:29:32 +0200,
songxiebing wrote:
> 
> From: Bob Song <songxiebing@kylinos.cn>
> 
> On some controllers the HDA link reports a verb timeout and the driver
> falls back to polling mode.  After that, every subsequent verb sent to
> that codec address keeps timing out (roughly one second per verb), while
> the hardware itself looks perfectly healthy: the RIRB write pointer keeps
> advancing, interrupts are still delivered, and verbs that carry no reply
> payload -- notably writes -- still take effect on the codec.
> 
> The reason is that bus->rirb.cmds[addr] is incremented for every verb and
> only decremented in snd_hdac_bus_update_rirb() when a matching RIRB
> response is read, without any rollback for a response that never arrives.
> A controller that cannot fall back to the single-command mode
> (chip->fallback_to_single_cmd == 0, e.g. the ACPI platform, Tegra and CIX
> controllers) returns an error right away and never performs the bus reset
> that would reinitialize CORB/RIRB and clear the counter, so a single lost
> response leaves bus->rirb.cmds[addr] non-zero forever.  Each following
> verb then still gets one response that only brings the count back to the
> stale baseline of 1, and snd_hdac_bus_get_response() keeps timing out even
> though the codec answers normally.
> 
> Handle it in the core helper snd_hdac_bus_exec_verb_unlocked(): whenever
> get_response() reports an error, reset the pending counter for the codec
> address, so a late response is simply treated as a spurious response and
> the following verbs recover.  Doing it here covers all HD-audio
> controllers instead of relying on each controller's fallback setting.
> 
> Add a small helper snd_hdac_bus_reset_response_counter() for this.
> 
> Signed-off-by: Bob Song <songxiebing@kylinos.cn>
> ---
> Changes in v2:
> - Move the fix from azx_rirb_get_response() in the HDA controller helper
>   into the HD-audio core (snd_hdac_bus_exec_verb_unlocked()), as suggested
>   by Takashi Iwai, so that it covers all HD-audio controllers and not only
>   the ones using the shared azx helper.
> - Add the snd_hdac_bus_reset_response_counter() helper instead of
>   open-coding the counter reset.
> 
>  include/sound/hdaudio.h     |  2 ++
>  sound/hda/core/bus.c        |  6 +++++-
>  sound/hda/core/controller.c | 20 ++++++++++++++++++++
>  3 files changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
> index aa994d6e6d35..b0fbb61fa093 100644
> --- a/include/sound/hdaudio.h
> +++ b/include/sound/hdaudio.h
> @@ -398,6 +398,8 @@ void snd_hdac_codec_link_down(struct hdac_device *codec);
>  int snd_hdac_bus_send_cmd(struct hdac_bus *bus, unsigned int val);
>  int snd_hdac_bus_get_response(struct hdac_bus *bus, unsigned int addr,
>  			      unsigned int *res);
> +void snd_hdac_bus_reset_response_counter(struct hdac_bus *bus,
> +					 unsigned int addr);
>  int snd_hdac_bus_parse_capabilities(struct hdac_bus *bus);
>  
>  bool snd_hdac_bus_init_chip(struct hdac_bus *bus, bool full_reset);
> diff --git a/sound/hda/core/bus.c b/sound/hda/core/bus.c
> index 81498f1e413e..ecc4b7a780a4 100644
> --- a/sound/hda/core/bus.c
> +++ b/sound/hda/core/bus.c
> @@ -121,11 +121,15 @@ int snd_hdac_bus_exec_verb_unlocked(struct hdac_bus *bus, unsigned int addr,
>  			break;
>  		/* process pending verbs */
>  		err = bus->ops->get_response(bus, addr, &tmp);
> -		if (err)
> +		if (err) {
> +			snd_hdac_bus_reset_response_counter(bus, addr);
>  			break;
> +		}
>  	}
>  	if (!err && res) {
>  		err = bus->ops->get_response(bus, addr, res);
> +		if (err)
> +			snd_hdac_bus_reset_response_counter(bus, addr);
>  		trace_hda_get_response(bus, addr, *res);
>  	}
>  	return err;
> diff --git a/sound/hda/core/controller.c b/sound/hda/core/controller.c
> index 78855ac357c6..1babc5109049 100644
> --- a/sound/hda/core/controller.c
> +++ b/sound/hda/core/controller.c
> @@ -396,6 +396,26 @@ int snd_hdac_bus_get_response(struct hdac_bus *bus, unsigned int addr,
>  }
>  EXPORT_SYMBOL_GPL(snd_hdac_bus_get_response);
>  
> +/**
> + * snd_hdac_bus_reset_response_counter - reset the pending response count
> + * @bus: HD-audio core bus
> + * @addr: codec address
> + *
> + * Drop the number of commands that are still waiting for a response from
> + * the given codec address.  This is called when a verb is known to be dead,
> + * e.g. after a response timeout, so that a lost response won't leave the
> + * counter non-zero forever and block all later verbs to the same codec.
> + */
> +void snd_hdac_bus_reset_response_counter(struct hdac_bus *bus, unsigned int addr)
> +{
> +	if (addr >= HDA_MAX_CODECS)
> +		return;
> +
> +	guard(spinlock_irq)(&bus->reg_lock);
> +	bus->rirb.cmds[addr] = 0;
> +}
> +EXPORT_SYMBOL_GPL(snd_hdac_bus_reset_response_counter);

Do we need to expose a symbol?  It's used in hda-core, and both bus.c
and controller.c are compiled together into it.
Let's avoid unneeded exports until really needed.


thanks,

Takashi

[PATCH v3] ALSA: hda: Reset response counter when a verb times out

[songxiebing]

From: Bob Song <songxiebing@kylinos.cn>

On some controllers the HDA link reports a verb timeout and the driver
falls back to polling mode.  After that, every subsequent verb sent to
that codec address keeps timing out (roughly one second per verb), while
the hardware itself looks perfectly healthy: the RIRB write pointer keeps
advancing, interrupts are still delivered, and verbs that carry no reply
payload -- notably writes -- still take effect on the codec.

The reason is that bus->rirb.cmds[addr] is incremented for every verb and
only decremented in snd_hdac_bus_update_rirb() when a matching RIRB
response is read, without any rollback for a response that never arrives.
A controller that cannot fall back to the single-command mode
(chip->fallback_to_single_cmd == 0, e.g. the ACPI platform, Tegra and CIX
controllers) returns an error right away and never performs the bus reset
that would reinitialize CORB/RIRB and clear the counter, so a single lost
response leaves bus->rirb.cmds[addr] non-zero forever.  Each following
verb then still gets one response that only brings the count back to the
stale baseline of 1, and snd_hdac_bus_get_response() keeps timing out even
though the codec answers normally.

Handle it in the core helper snd_hdac_bus_exec_verb_unlocked(): whenever
get_response() reports an error, reset the pending counter for the codec
address, so a late response is simply treated as a spurious response and
the following verbs recover.  Doing it here covers all HD-audio
controllers instead of relying on each controller's fallback setting.

Add a small helper snd_hdac_bus_reset_response_counter() for this.

Signed-off-by: Bob Song <songxiebing@kylinos.cn>
---
Changes in v3:
- Drop the unneeded EXPORT_SYMBOL_GPL() and move the declaration from
  include/sound/hdaudio.h into the internal core/local.h, since the helper
  is used only within the hda-core module.

Changes in v2:
- Move the fix from azx_rirb_get_response() in the HDA controller helper
  into the HD-audio core (snd_hdac_bus_exec_verb_unlocked()), as suggested
  by Takashi Iwai, so that it covers all HD-audio controllers and not only
  the ones using the shared azx helper.
- Add the snd_hdac_bus_reset_response_counter() helper instead of
  open-coding the counter reset.

 sound/hda/core/bus.c        |  6 +++++-
 sound/hda/core/controller.c | 19 +++++++++++++++++++
 sound/hda/core/local.h      |  2 ++
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/sound/hda/core/bus.c b/sound/hda/core/bus.c
index 81498f1e413e..ecc4b7a780a4 100644
--- a/sound/hda/core/bus.c
+++ b/sound/hda/core/bus.c
@@ -121,11 +121,15 @@ int snd_hdac_bus_exec_verb_unlocked(struct hdac_bus *bus, unsigned int addr,
 			break;
 		/* process pending verbs */
 		err = bus->ops->get_response(bus, addr, &tmp);
-		if (err)
+		if (err) {
+			snd_hdac_bus_reset_response_counter(bus, addr);
 			break;
+		}
 	}
 	if (!err && res) {
 		err = bus->ops->get_response(bus, addr, res);
+		if (err)
+			snd_hdac_bus_reset_response_counter(bus, addr);
 		trace_hda_get_response(bus, addr, *res);
 	}
 	return err;
diff --git a/sound/hda/core/controller.c b/sound/hda/core/controller.c
index 78855ac357c6..9993f5df9ee5 100644
--- a/sound/hda/core/controller.c
+++ b/sound/hda/core/controller.c
@@ -396,6 +396,25 @@ int snd_hdac_bus_get_response(struct hdac_bus *bus, unsigned int addr,
 }
 EXPORT_SYMBOL_GPL(snd_hdac_bus_get_response);
 
+/**
+ * snd_hdac_bus_reset_response_counter - reset the pending response count
+ * @bus: HD-audio core bus
+ * @addr: codec address
+ *
+ * Drop the number of commands that are still waiting for a response from
+ * the given codec address.  This is called when a verb is known to be dead,
+ * e.g. after a response timeout, so that a lost response won't leave the
+ * counter non-zero forever and block all later verbs to the same codec.
+ */
+void snd_hdac_bus_reset_response_counter(struct hdac_bus *bus, unsigned int addr)
+{
+	if (addr >= HDA_MAX_CODECS)
+		return;
+
+	guard(spinlock_irq)(&bus->reg_lock);
+	bus->rirb.cmds[addr] = 0;
+}
+
 #define HDAC_MAX_CAPS 10
 /**
  * snd_hdac_bus_parse_capabilities - parse capability structure
diff --git a/sound/hda/core/local.h b/sound/hda/core/local.h
index 5f03b203c416..b9e58395decb 100644
--- a/sound/hda/core/local.h
+++ b/sound/hda/core/local.h
@@ -16,6 +16,8 @@ int snd_hdac_bus_add_device(struct hdac_bus *bus, struct hdac_device *codec);
 void snd_hdac_bus_remove_device(struct hdac_bus *bus,
 				struct hdac_device *codec);
 void snd_hdac_bus_queue_event(struct hdac_bus *bus, u32 res, u32 res_ex);
+void snd_hdac_bus_reset_response_counter(struct hdac_bus *bus,
+					 unsigned int addr);
 int snd_hdac_bus_exec_verb(struct hdac_bus *bus, unsigned int addr,
 			   unsigned int cmd, unsigned int *res);
 
-- 
2.25.1

Re: [PATCH v3] ALSA: hda: Reset response counter when a verb times out

[Takashi Iwai]

On Sat, 10 Oct 2026 10:27:24 +0200,
songxiebing wrote:
> 
> From: Bob Song <songxiebing@kylinos.cn>
> 
> On some controllers the HDA link reports a verb timeout and the driver
> falls back to polling mode.  After that, every subsequent verb sent to
> that codec address keeps timing out (roughly one second per verb), while
> the hardware itself looks perfectly healthy: the RIRB write pointer keeps
> advancing, interrupts are still delivered, and verbs that carry no reply
> payload -- notably writes -- still take effect on the codec.
> 
> The reason is that bus->rirb.cmds[addr] is incremented for every verb and
> only decremented in snd_hdac_bus_update_rirb() when a matching RIRB
> response is read, without any rollback for a response that never arrives.
> A controller that cannot fall back to the single-command mode
> (chip->fallback_to_single_cmd == 0, e.g. the ACPI platform, Tegra and CIX
> controllers) returns an error right away and never performs the bus reset
> that would reinitialize CORB/RIRB and clear the counter, so a single lost
> response leaves bus->rirb.cmds[addr] non-zero forever.  Each following
> verb then still gets one response that only brings the count back to the
> stale baseline of 1, and snd_hdac_bus_get_response() keeps timing out even
> though the codec answers normally.
> 
> Handle it in the core helper snd_hdac_bus_exec_verb_unlocked(): whenever
> get_response() reports an error, reset the pending counter for the codec
> address, so a late response is simply treated as a spurious response and
> the following verbs recover.  Doing it here covers all HD-audio
> controllers instead of relying on each controller's fallback setting.
> 
> Add a small helper snd_hdac_bus_reset_response_counter() for this.
> 
> Signed-off-by: Bob Song <songxiebing@kylinos.cn>

Applied now to for-next branch.  Thanks.


Takashi