* [PATCH] drm/bridge: dw-hdmi-qp: Return -EOPNOTSUPP in HDMI audio functions
@ 2026-05-19 18:00 Detlev Casanova
2026-05-25 12:33 ` Claude review: " Claude Code Review Bot
2026-05-25 12:33 ` Claude Code Review Bot
0 siblings, 2 replies; 3+ messages in thread
From: Detlev Casanova @ 2026-05-19 18:00 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter
Cc: dri-devel, linux-kernel, kernel, Detlev Casanova
-EOPNOTSUPP is not logged as an error by the ASoC subsystem, but -ENODEV
is.
It also better represents the situation: The operation is currently not
supported (because clocks are not enabled and tmds_char_rate is
unavailable), but the hardware is present.
Using -EOPNOTSUPP in the audio_prepare callback removes possible repeated
warning log lines when HDMI is not connected.
Returning -EOPNOTSUPP in the audio_enable callback is also needed as it
avoids logging 0-valued ELD errors.
When tmds_char_rate is available, the clocks are enabled and the
functions will keep returning 0 as before.
Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
index d649a1cf07f5..270c8c47d853 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
@@ -469,8 +469,10 @@ static int dw_hdmi_qp_audio_enable(struct drm_bridge *bridge,
{
struct dw_hdmi_qp *hdmi = dw_hdmi_qp_from_bridge(bridge);
- if (hdmi->tmds_char_rate)
- dw_hdmi_qp_mod(hdmi, 0, AVP_DATAPATH_PACKET_AUDIO_SWDISABLE, GLOBAL_SWDISABLE);
+ if (!hdmi->tmds_char_rate)
+ return -EOPNOTSUPP;
+
+ dw_hdmi_qp_mod(hdmi, 0, AVP_DATAPATH_PACKET_AUDIO_SWDISABLE, GLOBAL_SWDISABLE);
return 0;
}
@@ -484,7 +486,7 @@ static int dw_hdmi_qp_audio_prepare(struct drm_bridge *bridge,
bool ref2stream = false;
if (!hdmi->tmds_char_rate)
- return -ENODEV;
+ return -EOPNOTSUPP;
if (fmt->bit_clk_provider | fmt->frame_clk_provider) {
dev_err(hdmi->dev, "unsupported clock settings\n");
---
base-commit: 1a2ab0feaa23147e347b4d4cb79cb3fc392118eb
change-id: 20260519-fix-hdmi-audio-warnings-139062ec00b9
Best regards,
--
Detlev Casanova <detlev.casanova@collabora.com>
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Claude review: drm/bridge: dw-hdmi-qp: Return -EOPNOTSUPP in HDMI audio functions
2026-05-19 18:00 [PATCH] drm/bridge: dw-hdmi-qp: Return -EOPNOTSUPP in HDMI audio functions Detlev Casanova
@ 2026-05-25 12:33 ` Claude Code Review Bot
2026-05-25 12:33 ` Claude Code Review Bot
1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-05-25 12:33 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/bridge: dw-hdmi-qp: Return -EOPNOTSUPP in HDMI audio functions
Author: Detlev Casanova <detlev.casanova@collabora.com>
Patches: 1
Reviewed: 2026-05-25T22:33:16.962963
---
This is a single-patch series that changes the error code returned by the `dw_hdmi_qp_audio_enable` and `dw_hdmi_qp_audio_prepare` callbacks from `-ENODEV` (and implicit success-when-not-ready) to `-EOPNOTSUPP` when `tmds_char_rate == 0` (i.e., HDMI is not connected).
The change is well-motivated and correct. The ASoC framework's `snd_soc_ret()` function (`sound/soc/soc-utils.c:18-42`) explicitly suppresses logging for `-EOPNOTSUPP` and `-ENOTSUPP`, while all other negative error codes (including the previous `-ENODEV`) produce `dev_err` messages. This means the old code was generating spurious error logs whenever audio operations were attempted while HDMI was disconnected — a normal and frequent runtime condition.
The semantic choice is also correct: `-EOPNOTSUPP` ("operation not supported") accurately describes a transient state where the hardware exists but clocks aren't enabled yet, whereas `-ENODEV` ("no such device") incorrectly suggests the hardware is absent.
**Verdict: Looks good. Suitable for merging.**
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 3+ messages in thread
* Claude review: drm/bridge: dw-hdmi-qp: Return -EOPNOTSUPP in HDMI audio functions
2026-05-19 18:00 [PATCH] drm/bridge: dw-hdmi-qp: Return -EOPNOTSUPP in HDMI audio functions Detlev Casanova
2026-05-25 12:33 ` Claude review: " Claude Code Review Bot
@ 2026-05-25 12:33 ` Claude Code Review Bot
1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-05-25 12:33 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**audio_enable refactoring (lines 472-475 after patch):**
```c
if (!hdmi->tmds_char_rate)
return -EOPNOTSUPP;
dw_hdmi_qp_mod(hdmi, 0, AVP_DATAPATH_PACKET_AUDIO_SWDISABLE, GLOBAL_SWDISABLE);
```
Previously, the logic was:
```c
if (hdmi->tmds_char_rate)
dw_hdmi_qp_mod(hdmi, 0, AVP_DATAPATH_PACKET_AUDIO_SWDISABLE, GLOBAL_SWDISABLE);
return 0;
```
The old code returned success (0) even when `tmds_char_rate` was 0 — meaning audio was "enabled" successfully without actually touching the hardware. This is a subtle behavioral change: callers (the `hdmi_codec_startup` path) now see a failure when HDMI is disconnected, rather than a silent no-op success. This is the correct fix — a silent success would allow the ASoC stack to proceed to `get_eld` and `prepare`, which would operate on stale/zero ELD data. By returning `-EOPNOTSUPP` here, the startup aborts cleanly before the ELD path (`hdmi-codec.c:468-482`) is reached, which prevents the "0-valued ELD errors" mentioned in the commit message.
**audio_prepare change (line 489 after patch):**
```c
if (!hdmi->tmds_char_rate)
return -EOPNOTSUPP;
```
This is a straightforward error code substitution from `-ENODEV` to `-EOPNOTSUPP`. The guard already existed; only the error code changes.
**Consistency with audio_disable:**
The `dw_hdmi_qp_audio_disable` function (line 526) returns `void` and already has a `tmds_char_rate` guard that simply skips the register writes. No change needed there, and the patch correctly doesn't touch it.
**No issues found.** The patch is minimal, well-scoped, and the commit message accurately explains both the problem and the mechanism. The `-EOPNOTSUPP` choice is validated by `snd_soc_ret()` in `sound/soc/soc-utils.c:30-31` which explicitly skips `dev_err` logging for this error code.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-05-25 12:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-19 18:00 [PATCH] drm/bridge: dw-hdmi-qp: Return -EOPNOTSUPP in HDMI audio functions Detlev Casanova
2026-05-25 12:33 ` Claude review: " Claude Code Review Bot
2026-05-25 12:33 ` Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox