drm/bridge: it6505: DP audio support + shared-DAI hw_params fix

PATCH Superseded Daniel Golle 3 participants 25 messages Jul 25 – Aug 31

Subsystems (4)

DRM drivers DRM misc Arm MediaTek DRM drivers for bridge chips

lore

Share & tools

lore

Daniel Golle

author

2026-07-25 02:58 UTC #0 lore raw

v6 cover drm/bridge: it6505: DP audio support + shared-DAI hw_params fix

Revive HDMI/DP audio support for the it6505, originally posted by Jiaxin Yu [1], rebased onto current -next with the review feedback
addressed: &client->dev is gone (the driver gained it6505->dev) and "#sound-dai-cells" has since been added by commit 325af1bef5b9 ("dt-bindings: display: bridge: it6505: Add #sound-dai-cells").

Patches 1-11 fix pre-existing bugs found during review of earlier versions of this series; they carry Fixes: tags and are ordered first so stable trees can pick them.

Patch 12 wires up the it6505 audio helpers via hdmi_codec_ops, unblocking the mt8186-mt6366 sound card which references it6505 as the I2S3 codec. Patch 13 makes hw_params succeed with no display attached, needed when the I2S bus is shared with another codec as on MT8186 "steelix" Chromebooks, where the speaker path must keep working regardless of the it6505 link state.

Tested on a MT8186 (google,steelix) Chromebook: the sound card comes up, speakers, microphone and headset jack work. HDMI/DP audio output itself could not be tested for lack of a suitable adapter cable.

[1] https://lore.kernel.org/all/[email protected]/
---
v6:
* patch 1: track notifier registration in a flag instead of clearing notifier_call, which a chain traversal racing the unregistration could have called as NULL (extcon traverses its raw notifier chain unlocked); flagged by Sashiko AI review of v5. The in-flight-callback race also flagged there is an extcon core limitation affecting all its consumers and cannot be closed from a consumer driver.
* patch 4: note that a failed regulator_disable() leaks the reference either way
* new patches 10 and 11: two more pre-existing fixes from v5 review, an uninitialised link-frequencies value and an error code written back to the reset register
* patches 2, 3, 5-9, 12 and 13: no changes

v5: https://lore.kernel.org/all/[email protected]/
* patch 1: serialise notifier registration state with extcon_lock
* patch 2: also drop the runtime PM usage ref held for a connected display; retitled
* new patches 3-7 and 9: more pre-existing fixes from v4 review (AUX unregister symmetry, poweroff error path, IRQ status read errors, two divisions by zero, of_node_put before use)
* patch 10 (was 4): update plugged_cb/codec_dev under mode_lock
* patches 8 and 11: no changes

v4: https://lore.kernel.org/all/[email protected]/
* patch 1: quiesce extcon notifier and IRQ before cancelling work, initialise extcon_wq in probe; retitled
* new patch 2: disable runtime PM on remove
* patch 3 (was 2): collected Chen-Yu Tsai's Reviewed-by
* patch 4 (was 3): start out muted; mute state and cached parameters updated under audio_lock; unregister codec after quiesce
* patch 5 (was 4): apply audio defaults at probe time

v3: https://lore.kernel.org/all/[email protected]/
* new patches 1 and 2, fixing a pre-existing use-after-free on remove() and an out-of-bounds read with channel_count == 0
* patch 3 (was 1): serialise audio enable/disable with audio_lock; track mute state so the FIFO-error IRQ cannot undo a mute
* patch 4 (was 2): no changes

v2: https://lore.kernel.org/all/[email protected]/
* keep and unregister the hdmi-codec platform_device on remove
* initialise delayed_audio before registering the codec device
* cancel delayed_audio synchronously on shutdown and remove
* disable audio immediately when muting
* patch 2: drop the encoder check instead of returning 0 early

v1: https://lore.kernel.org/all/[email protected]/

Daniel Golle (12):
drm/bridge: it6505: quiesce event sources and work on remove() drm/bridge: it6505: balance and disable runtime PM on remove drm/bridge: it6505: unregister DP AUX adapter on bridge detach drm/bridge: it6505: complete poweroff even if disabling regulators
fails
drm/bridge: it6505: bail out of the IRQ handler when status reads fail drm/bridge: it6505: avoid division by zero in pixel clock calculation drm/bridge: it6505: avoid division by zero in audio FS debug print drm/bridge: it6505: guard against zero channel count in audio
infoframe
drm/bridge: it6505: hold endpoint OF node reference while parsing it drm/bridge: it6505: reject a too short link-frequencies property drm/bridge: it6505: don't write an error code back to the reset
register
drm/bridge: it6505: Don't reject audio hw_params without an encoder

Jiaxin Yu (1):
drm/bridge: it6505: Add audio support

 drivers/gpu/drm/bridge/ite-it6505.c | 256 +++++++++++++++++++++-------
 1 file changed, 197 insertions(+), 59 deletions(-)

base-commit: 9eebf259d5352b87080d67758f483583d9e763d7

Signature · 1 line

2.55.0

Daniel Golle

author

2026-07-25 02:58 UTC #1 lore raw

PATCH v6 1/13 drm/bridge: it6505: quiesce event sources and work on remove()

struct it6505 is freed by devres right after remove() returns, but remove() cancels none of the driver's work items, so link_works, hdcp_wait_ksv_list, hdcp_work and extcon_wq can still run afterwards and dereference freed memory. Cancelling alone would not be enough:
the threaded IRQ and the extcon notifier stay live until devres teardown and can requeue the works.

Unregister the extcon notifier and disable the IRQ first, then cancel all work. Make it6505_remove_notifier_module() idempotent, tracking the registration in a flag under extcon_lock, as both .detach() and remove() call it now. The notifier_call pointer is left intact:
extcon traverses its raw notifier chain without holding a lock, so a traversal racing with the unregistration may still invoke the callback. Also initialise extcon_wq in probe: cancel_work_sync() on a never-initialised work item trips WARN_ON(!work->func).

Fixes b5c84a9edcd4 ("drm/bridge: add it6505 driver")

Signed-off-by Daniel Golle <[email protected]>

---
v6: track registration in a flag instead of clearing notifier_call,
which an unlocked chain traversal racing the unregistration could have called as NULL

v5: serialise notifier registration state with extcon_lock

v4:
* quiesce event sources before cancelling work; retitled
* initialise extcon_wq in probe to avoid WARN_ON(!work->func)

v3: new patch

 drivers/gpu/drm/bridge/ite-it6505.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

drivers/gpu/drm/bridge/ite-it6505.c +21 -6

index 8ecb43611dba..a7ba6a0befda 100644
@@ -443,6 +443,7 @@ struct it6505 {
 	struct drm_display_mode source_output_mode;
 	struct drm_display_mode video_info;
 	struct notifier_block event_nb;
+	bool extcon_registered;
 	struct extcon_dev *extcon;
 	struct work_struct extcon_wq;
 	int extcon_state;

@@ -2934,11 +2935,15 @@ static int it6505_use_notifier_module(struct it6505 *it6505)
 	int ret;
 	struct device *dev = it6505->dev;
 
+	mutex_lock(&it6505->extcon_lock);
 	it6505->event_nb.notifier_call = it6505_extcon_notifier;
-	INIT_WORK(&it6505->extcon_wq, it6505_extcon_work);
 	ret = devm_extcon_register_notifier(it6505->dev,
 					    it6505->extcon, EXTCON_DISP_DP,
 					    &it6505->event_nb);
+	if (!ret)
+		it6505->extcon_registered = true;
+	mutex_unlock(&it6505->extcon_lock);
+
 	if (ret) {
 		dev_err(dev, "failed to register notifier for DP");
 		return ret;

@@ -2951,13 +2956,16 @@ static int it6505_use_notifier_module(struct it6505 *it6505)
 
 static void it6505_remove_notifier_module(struct it6505 *it6505)
 {
-	if (it6505->extcon) {
-		devm_extcon_unregister_notifier(it6505->dev,
-						it6505->extcon,	EXTCON_DISP_DP,
+	mutex_lock(&it6505->extcon_lock);
+	if (it6505->extcon && it6505->extcon_registered) {
+		devm_extcon_unregister_notifier(it6505->dev, it6505->extcon,
+						EXTCON_DISP_DP,
 						&it6505->event_nb);
-
-		flush_work(&it6505->extcon_wq);
+		it6505->extcon_registered = false;
 	}
+	mutex_unlock(&it6505->extcon_lock);
+
+	flush_work(&it6505->extcon_wq);
 }
 
 static void __maybe_unused it6505_delayed_audio(struct work_struct *work)

@@ -3615,6 +3623,7 @@ static int it6505_i2c_probe(struct i2c_client *client)
 	INIT_WORK(&it6505->link_works, it6505_link_training_work);
 	INIT_WORK(&it6505->hdcp_wait_ksv_list, it6505_hdcp_wait_ksv_list);
 	INIT_DELAYED_WORK(&it6505->hdcp_work, it6505_hdcp_work);
+	INIT_WORK(&it6505->extcon_wq, it6505_extcon_work);
 	init_completion(&it6505->extcon_completion);
 	memset(it6505->dpcd, 0, sizeof(it6505->dpcd));
 	it6505->powered = false;

@@ -3647,6 +3656,12 @@ static void it6505_i2c_remove(struct i2c_client *client)
 	drm_bridge_remove(&it6505->bridge);
 	drm_dp_aux_unregister(&it6505->aux);
 	it6505_debugfs_remove(it6505);
+	it6505_remove_notifier_module(it6505);
+	disable_irq(it6505->irq);
+	cancel_work_sync(&it6505->link_works);
+	cancel_work_sync(&it6505->hdcp_wait_ksv_list);
+	cancel_delayed_work_sync(&it6505->hdcp_work);
+	cancel_work_sync(&it6505->extcon_wq);
 	it6505_poweroff(it6505);
 	it6505_remove_edid(it6505);
 }

Signature · 1 line

2.55.0

[email protected]

AI review

2026-07-25 03:23 UTC #2 lore raw

Re: [PATCH v6 01/13] drm/bridge: it6505: quiesce event sources and work on remove()

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] Use-After-Free in extcon_wq due to raw notifier traversal race during driver removal.
- [High] NULL Pointer Dereference in it6505_extcon_work when racing with bridge detach.
- [Medium] Data race on extcon_completion state between reinit_completion and wait_for_completion_timeout.
- [Medium] Missing pm_runtime_disable() in the I2C driver remove path.
--

--- Patch [1]: [PATCH v6 01/13] drm/bridge: it6505: quiesce event sources and work on remove() --- commit ec8b6db3474d25da1dc993de7833610db7cf6509
Author: Daniel Golle <[email protected]>

drm/bridge: it6505: quiesce event sources and work on remove()

This patch attempts to prevent use-after-free and NULL pointer dereference issues during driver removal by unregistering the extcon notifier, disabling interrupts, and explicitly cancelling work items. It also tracks the extcon registration state to make unregistration idempotent.

Quoted 3 lines

drivers/gpu/drm/bridge/ite-it6505.c +0 -0

[ ... ]

Quoted 10 lines

drivers/gpu/drm/bridge/ite-it6505.c +3 -3

@@ -2951,13 +2956,16 @@ static int it6505_use_notifier_module(struct it6505 *it6505)
 
 static void it6505_remove_notifier_module(struct it6505 *it6505)
 {
-	if (it6505->extcon) {
-		devm_extcon_unregister_notifier(it6505->dev,
-						it6505->extcon,	EXTCON_DISP_DP,
+	mutex_lock(&it6505->extcon_lock);
+	if (it6505->extcon && it6505->extcon_registered) {
+		devm_extcon_unregister_notifier(it6505->dev, it6505->extcon,

drivers/gpu/drm/bridge/ite-it6505.c static int it6505_use_notifier_module(struct it6505 *it6505) · line 2969

Quoted 10 lines

drivers/gpu/drm/bridge/ite-it6505.c +5 -2

+						EXTCON_DISP_DP,
 						&it6505->event_nb);
-
-		flush_work(&it6505->extcon_wq);
+		it6505->extcon_registered = false;
 	}
+	mutex_unlock(&it6505->extcon_lock);
+
+	flush_work(&it6505->extcon_wq);
 }

[Severity: High]
This isn't a bug introduced by this patch, but does unregistering the extcon notifier fully prevent the work item from being requeued?

Because devm_extcon_unregister_notifier() unlinks the notifier from a raw notifier chain, it cannot synchronize against a concurrent traversal.

If a traversal thread has already read the node but not yet invoked the callback, it can execute after this flush_work() completes:

it6505_extcon_notifier()

    schedule_work(&it6505->extcon_wq);

If it6505_i2c_remove() then cancels the work and devres frees the it6505 struct, the preempted traversal thread could resume and requeue the work. Is there a way this could result in a use-after-free on the freed memory?

[Severity: High]
This is a pre-existing issue, but could this same raw notifier race also lead to a NULL pointer dereference if it races with bridge detach?

During drm_bridge_detach(), the core sets bridge->dev = NULL. If the delayed traversal requeues it6505_extcon_work() as described above, the requeued work will eventually execute:

it6505_extcon_work()

    drm_helper_hpd_irq_event(it6505->bridge.dev);

Will this dereference a NULL pointer since bridge->dev was already cleared?

[ ... ]

drivers/gpu/drm/bridge/ite-it6505.c static int it6505_i2c_probe(struct i2c_client *client) · line 3627

Quoted 6 lines

drivers/gpu/drm/bridge/ite-it6505.c +1 -0

@@ -3615,6 +3623,7 @@ static int it6505_i2c_probe(struct i2c_client *client)
 	INIT_WORK(&it6505->link_works, it6505_link_training_work);
 	INIT_WORK(&it6505->hdcp_wait_ksv_list, it6505_hdcp_wait_ksv_list);
 	INIT_DELAYED_WORK(&it6505->hdcp_work, it6505_hdcp_work);
+	INIT_WORK(&it6505->extcon_wq, it6505_extcon_work);
 	init_completion(&it6505->extcon_completion);

[Severity: Medium]
This is a pre-existing issue, but is it safe to use reinit_completion() in it6505_extcon_work() without synchronization?

The it6505_irq_hpd() handler actively waits on this completion:

it6505_irq_hpd()

    wait_for_completion_timeout(&it6505->extcon_completion, ...)

If an extcon event occurs concurrently, it6505_extcon_work() calls reinit_completion() directly on it6505->extcon_completion, which resets the completion state without acquiring the internal wait.lock.

Can this corrupt the completion state or cause the waiter to time out incorrectly?

Quoted 5 lines

drivers/gpu/drm/bridge/ite-it6505.c +0 -0

 	memset(it6505->dpcd, 0, sizeof(it6505->dpcd));
 	it6505->powered = false;

@@ -3647,6 +3656,12 @@ static void it6505_i2c_remove(struct i2c_client *client)
 	drm_bridge_remove(&it6505->bridge);
 	drm_dp_aux_unregister(&it6505->aux);

drivers/gpu/drm/bridge/ite-it6505.c static void it6505_i2c_remove(struct i2c_client *client) · line 3667

Quoted 10 lines

drivers/gpu/drm/bridge/ite-it6505.c +6 -0

 	it6505_debugfs_remove(it6505);
+	it6505_remove_notifier_module(it6505);
+	disable_irq(it6505->irq);
+	cancel_work_sync(&it6505->link_works);
+	cancel_work_sync(&it6505->hdcp_wait_ksv_list);
+	cancel_delayed_work_sync(&it6505->hdcp_work);
+	cancel_work_sync(&it6505->extcon_wq);
 	it6505_poweroff(it6505);
 	it6505_remove_edid(it6505);
 }

[Severity: Medium]
This is a pre-existing issue, but is the remove path missing a call to pm_runtime_disable()?

The it6505_i2c_probe() function explicitly calls pm_runtime_enable(), but there doesn't appear to be a corresponding disable in this it6505_i2c_remove() function.

Does leaving PM runtime enabled after driver unbind leave the device in an incorrect state or cause reference count leaks?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Daniel Golle

author

2026-07-25 02:58 UTC #3 lore raw

PATCH v6 2/13 drm/bridge: it6505: balance and disable runtime PM on remove

In reply to Daniel Golle

Runtime PM is enabled in probe but never disabled on remove, so rebinding the driver warns "Unbalanced pm_runtime_enable!". The extcon work also holds a usage reference while a display is connected, which unbinding leaks; the count survives in struct device, so after a rebind the device never runtime-suspends again.

Drop the usage reference held for a connected display and disable runtime PM in remove().

Fixes 10517777d302 ("drm/bridge: it6505: Adapt runtime power management framework")

Signed-off-by Daniel Golle <[email protected]>

---
v6: no changes.

v5: also drop the usage ref held for a connected display; retitled

v4: new patch

 drivers/gpu/drm/bridge/ite-it6505.c | 3 +++
 1 file changed, 3 insertions(+)

drivers/gpu/drm/bridge/ite-it6505.c +3 -0

index a7ba6a0befda..7c61b02cc7e6 100644
@@ -3662,6 +3662,9 @@ static void it6505_i2c_remove(struct i2c_client *client)
 	cancel_work_sync(&it6505->hdcp_wait_ksv_list);
 	cancel_delayed_work_sync(&it6505->hdcp_work);
 	cancel_work_sync(&it6505->extcon_wq);
+	if (it6505->extcon_state)
+		pm_runtime_put_sync(&client->dev);
+	pm_runtime_disable(&client->dev);
 	it6505_poweroff(it6505);
 	it6505_remove_edid(it6505);
 }

Signature · 1 line

2.55.0

[email protected]

AI review

2026-07-25 03:18 UTC #4 lore raw

Re: [PATCH v6 02/13] drm/bridge: it6505: balance and disable runtime PM on remove

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unbalanced Lifecycle Operations: drm_dp_aux_register is called during bridge attach, but unregistered during driver remove. --

--- Patch [2]: [PATCH v6 02/13] drm/bridge: it6505: balance and disable runtime PM on remove --- commit ff66d772f6681026b2d4511d26d9b73d925d8240
Author: Daniel Golle <[email protected]>

drm/bridge: it6505: balance and disable runtime PM on remove

This commit disables runtime PM in the remove function and drops the usage reference held for a connected display, fixing an unbalanced runtime PM warning that occurs upon rebinding the driver.

drivers/gpu/drm/bridge/ite-it6505.c static void it6505_i2c_remove(struct i2c_client *client)

Quoted 4 lines

drivers/gpu/drm/bridge/ite-it6505.c +0 -0
@@ -3662,6 +3662,9 @@ static void it6505_i2c_remove(struct i2c_client *client)

[Severity: High]
This isn't a bug introduced by this patch, but does this driver have an unbalanced lifecycle for the AUX channel adapter?

Registration happens during bridge attach:

it6505_bridge_attach() {
        ...
        ret = drm_dp_aux_register(&it6505->aux);
        ...
}

However, the unregistration happens just above this diff context in it6505_i2c_remove() rather than in it6505_bridge_detach():

it6505_i2c_remove() {
        ...
        drm_dp_aux_unregister(&it6505->aux);
        ...
}

If the DRM driver or device undergoes an unbind and rebind cycle, the detach and attach callbacks will execute without the underlying I2C driver being removed.

Could this lead to drm_dp_aux_register() being called a second time without a prior unregister, causing it to fail with -EEXIST since the adapter is already registered?

Quoted 9 lines

drivers/gpu/drm/bridge/ite-it6505.c +3 -0

 	cancel_work_sync(&it6505->hdcp_wait_ksv_list);
 	cancel_delayed_work_sync(&it6505->hdcp_work);
 	cancel_work_sync(&it6505->extcon_wq);
+	if (it6505->extcon_state)
+		pm_runtime_put_sync(&client->dev);
+	pm_runtime_disable(&client->dev);
 	it6505_poweroff(it6505);
 	it6505_remove_edid(it6505);
 }

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2

Daniel Golle

author

2026-07-25 02:58 UTC #5 lore raw

PATCH v6 3/13 drm/bridge: it6505: unregister DP AUX adapter on bridge detach

In reply to Daniel Golle

The DP AUX adapter is registered in .attach() but only unregistered in i2c remove(). Unbinding and rebinding the DRM device while the i2c driver stays bound registers the adapter a second time, which fails. Unregister it in .detach() instead, and also on the .attach() error path, which so far leaked the registration.

Fixes b5c84a9edcd4 ("drm/bridge: add it6505 driver")

Signed-off-by Daniel Golle <[email protected]>

---
v6: no changes.

v5: new patch

 drivers/gpu/drm/bridge/ite-it6505.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/gpu/drm/bridge/ite-it6505.c +2 -1

index 7c61b02cc7e6..c10cc6a786b5 100644
@@ -3093,6 +3093,7 @@ static int it6505_bridge_attach(struct drm_bridge *bridge,
 		ret = it6505_use_notifier_module(it6505);
 		if (ret < 0) {
 			dev_err(dev, "use notifier module failed");
+			drm_dp_aux_unregister(&it6505->aux);
 			return ret;
 		}
 	}

@@ -3106,6 +3107,7 @@ static void it6505_bridge_detach(struct drm_bridge *bridge)
 
 	flush_work(&it6505->link_works);
 	it6505_remove_notifier_module(it6505);
+	drm_dp_aux_unregister(&it6505->aux);
 }
 
 static enum drm_mode_status

@@ -3654,7 +3656,6 @@ static void it6505_i2c_remove(struct i2c_client *client)
 	struct it6505 *it6505 = i2c_get_clientdata(client);
 
 	drm_bridge_remove(&it6505->bridge);
-	drm_dp_aux_unregister(&it6505->aux);
 	it6505_debugfs_remove(it6505);
 	it6505_remove_notifier_module(it6505);
 	disable_irq(it6505->irq);

Signature · 1 line

2.55.0

[email protected]

AI review

2026-07-25 03:17 UTC #6 lore raw

Re: [PATCH v6 03/13] drm/bridge: it6505: unregister DP AUX adapter on bridge detach

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Removing `drm_dp_aux_unregister()` from the I2C `remove` function causes a use-after-free and…

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing drm_dp_aux_unregister() from the I2C remove function causes a use-after-free and leaves a zombie DP AUX adapter if the I2C driver is unbound while the DRM device remains attached. --

--- Patch [3]: [PATCH v6 03/13] drm/bridge: it6505: unregister DP AUX adapter on bridge detach --- commit d6c69e9e1c11461da60c039acb301f1120930d47
Author: Daniel Golle <[email protected]>

drm/bridge: it6505: unregister DP AUX adapter on bridge detach

This commit moves the DP AUX adapter unregistration from the I2C remove routine to the DRM bridge detach and attach error paths, to avoid registering the adapter multiple times during unbind/rebind cycles.

Quoted 4 lines

drivers/gpu/drm/bridge/ite-it6505.c +0 -0
index 7c61b02cc7e60..c10cc6a786b5d 100644

[ ... ]

Quoted 7 lines

drivers/gpu/drm/bridge/ite-it6505.c +1 -0

@@ -3106,6 +3107,7 @@ static void it6505_bridge_detach(struct drm_bridge *bridge)
 
 	flush_work(&it6505->link_works);
 	it6505_remove_notifier_module(it6505);
+	drm_dp_aux_unregister(&it6505->aux);
 }

[ ... ]

drivers/gpu/drm/bridge/ite-it6505.c static void it6505_i2c_remove(struct i2c_client *client) · line 3657

Quoted 5 lines

drivers/gpu/drm/bridge/ite-it6505.c +0 -1

@@ -3654,7 +3656,6 @@ static void it6505_i2c_remove(struct i2c_client *client)
 	struct it6505 *it6505 = i2c_get_clientdata(client);
 
 	drm_bridge_remove(&it6505->bridge);
-	drm_dp_aux_unregister(&it6505->aux);

[Severity: High]
Does removing drm_dp_aux_unregister() here cause a use-after-free if the I2C driver is unbound while the DRM device is still attached?

If the I2C driver is unbound (e.g., via sysfs) while the DRM display controller is still bound, it6505_i2c_remove() is called. The drm_bridge_remove() call only moves the bridge to a lingering list and does not immediately call it6505_bridge_detach().

Because drm_dp_aux_unregister() was removed from this path, the AUX adapter remains registered. When the remove function returns, devres frees the it6505 structure.

Could a local user trigger a use-after-free by accessing the leaked /dev/drm_dp_auxN character device or I2C sysfs nodes which now point to the freed structure?

Since i2c_del_adapter() and drm_dp_aux_unregister_devnode() both contain safety guards against unregistered adapters, is it safer to leave drm_dp_aux_unregister() in the remove path as well to ensure it is always unregistered before devres frees the memory?

Quoted 3 lines

drivers/gpu/drm/bridge/ite-it6505.c +0 -0

 	it6505_debugfs_remove(it6505);
 	it6505_remove_notifier_module(it6505);
 	disable_irq(it6505->irq);

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3

Daniel Golle

author

2026-07-25 02:58 UTC #7 lore raw

PATCH v6 4/13 drm/bridge: it6505: complete poweroff even if disabling regulators fails

In reply to Daniel Golle

it6505_poweroff() returns early when regulator_disable() fails, leaving it6505->powered set with the IRQ already disabled. The next it6505_poweron() then takes its early return and never re-enables the IRQ, leaving the bridge deaf to hotplug and link training interrupts. The regulator core keeps the consumer's enable count on a failed disable either way, so bailing out only adds a wedged bridge on top of the leaked reference. Log the error and complete the power-off state transition instead.

Fixes b5c84a9edcd4 ("drm/bridge: add it6505 driver")

Signed-off-by Daniel Golle <[email protected]>

---
v6: note that the regulator reference is leaked either way

v5: new patch

 drivers/gpu/drm/bridge/ite-it6505.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

drivers/gpu/drm/bridge/ite-it6505.c +4 -2

index c10cc6a786b5..fb6030a2c18c 100644
@@ -2811,13 +2811,15 @@ static int it6505_poweroff(struct it6505 *it6505)
 	if (pdata->pwr18) {
 		err = regulator_disable(pdata->pwr18);
 		if (err)
-			return err;
+			dev_err(dev, "cannot disable pwr18 regulator: %d",
+				err);
 	}
 
 	if (pdata->ovdd) {
 		err = regulator_disable(pdata->ovdd);
 		if (err)
-			return err;
+			dev_err(dev, "cannot disable ovdd regulator: %d",
+				err);
 	}
 
 	it6505->powered = false;

Signature · 1 line

2.55.0

[email protected]

AI review

2026-07-25 03:18 UTC #8 lore raw

Re: [PATCH v6 04/13] drm/bridge: it6505: complete poweroff even if disabling regulators fails

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Low] Missing newline (\n) in the newly added dev_err format strings.

Pre-existing issues:
- [High] The interrupt handler does not check the return value of it6505_read() for error codes, causing spurious interrupt triggers.
- [High] The power-off sequence disables the IVDD (pwr18) regulator before the OVDD (ovdd) regulator, violating the reverse-order power-down requirement. --

--- Patch [4]: [PATCH v6 04/13] drm/bridge: it6505: complete poweroff even if disabling regulators fails --- commit 1009be5a50c31876cc2fed6f2166f7843b4776a3
Author: Daniel Golle <[email protected]>

drm/bridge: it6505: complete poweroff even if disabling regulators fails

drivers/gpu/drm/bridge/ite-it6505.c static int it6505_poweroff(struct it6505 *it6505) · line 2812

Quoted 7 lines

drivers/gpu/drm/bridge/ite-it6505.c +0 -0

index c10cc6a786b5d..fb6030a2c18cb 100644
@@ -2811,13 +2811,15 @@ static int it6505_poweroff(struct it6505 *it6505)
 	if (pdata->pwr18) {
 		err = regulator_disable(pdata->pwr18);

[Severity: High]
This isn't a bug introduced by this patch, but does this power-off sequence violate the reverse-order power-down requirement?

The it6505_poweron() function explicitly enforces pwr18 -> ovdd ordering, but this disables the core voltage (IVDD/pwr18) before the IO voltage (OVDD/ovdd).

Could this forward order create reverse current paths through ESD diodes, leading to hardware latch-up or degradation?

Should we disable ovdd before pwr18 here?

drivers/gpu/drm/bridge/ite-it6505.c static int it6505_poweroff(struct it6505 *it6505) · line 2815

Quoted 4 lines

drivers/gpu/drm/bridge/ite-it6505.c +2 -1

 		if (err)
-			return err;
+			dev_err(dev, "cannot disable pwr18 regulator: %d",
+				err);

[Severity: Low]
Is this missing a trailing newline?

Without a \n in the format string, subsequent kernel log messages from other subsystems might be concatenated on the same line, resulting in mangled dmesg output.

drivers/gpu/drm/bridge/ite-it6505.c static int it6505_poweroff(struct it6505 *it6505) · line 2822

Quoted 8 lines

drivers/gpu/drm/bridge/ite-it6505.c +2 -1

 	}
 
 	if (pdata->ovdd) {
 		err = regulator_disable(pdata->ovdd);
 		if (err)
-			return err;
+			dev_err(dev, "cannot disable ovdd regulator: %d",
+				err);

[Severity: Low]
Does this dev_err() also need a trailing newline?

drivers/gpu/drm/bridge/ite-it6505.c static int it6505_poweroff(struct it6505 *it6505) · line 2825

Quoted 3 lines

drivers/gpu/drm/bridge/ite-it6505.c +0 -0
 	}
 
 	it6505->powered = false;

[Severity: High]
This is a pre-existing issue, but does it6505_int_threaded_handler() fail to check it6505_read() for error codes?

If an I2C read fails due to a transient bus error or occurring during power-down, it6505_read() returns a negative error code (e.g., -ENODEV).

This negative value is directly assigned to the int_status array:

drivers/gpu/drm/bridge/ite-it6505.c:it6505_int_threaded_handler() {
	int_status[0] = it6505_read(it6505, INT_STATUS_01);
	int_status[1] = it6505_read(it6505, INT_STATUS_02);
	int_status[2] = it6505_read(it6505, INT_STATUS_03);

...
}

In two's complement, negative values have many high bits set. This causes it6505_test_bit() to incorrectly evaluate to true for multiple random interrupts.

Could this lead to the spurious execution of handlers and writing back garbage to the interrupt clear register?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4

Daniel Golle

author

2026-07-25 02:59 UTC #9 lore raw

PATCH v6 5/13 drm/bridge: it6505: bail out of the IRQ handler when status reads fail

In reply to Daniel Golle

When reading the interrupt status registers fails, the negative error codes end up in int_status[], where it6505_test_bit() sees almost all bits set: every interrupt sub-handler runs on garbage and the error values are even written back to the status registers. Return IRQ_NONE instead.

Fixes b5c84a9edcd4 ("drm/bridge: add it6505 driver")

Signed-off-by Daniel Golle <[email protected]>

---
v6: no changes

v5: new patch

 drivers/gpu/drm/bridge/ite-it6505.c | 5 +++++
 1 file changed, 5 insertions(+)

drivers/gpu/drm/bridge/ite-it6505.c +5 -0

index fb6030a2c18c..3806b0b96637 100644
@@ -2714,6 +2714,11 @@ static irqreturn_t it6505_int_threaded_handler(int unused, void *data)
 	int_status[1] = it6505_read(it6505, INT_STATUS_02);
 	int_status[2] = it6505_read(it6505, INT_STATUS_03);
 
+	if (int_status[0] < 0 || int_status[1] < 0 || int_status[2] < 0) {
+		pm_runtime_put_sync(dev);
+		return IRQ_NONE;
+	}
+
 	it6505_write(it6505, INT_STATUS_01, int_status[0]);
 	it6505_write(it6505, INT_STATUS_02, int_status[1]);
 	it6505_write(it6505, INT_STATUS_03, int_status[2]);

Signature · 1 line

2.55.0

Daniel Golle

author

2026-07-25 02:59 UTC #10 lore raw

PATCH v6 6/13 drm/bridge: it6505: avoid division by zero in pixel clock calculation

In reply to Daniel Golle

it6505_calc_video_info() checks the sum of the three pixel clock counter samples for zero before dividing it by 3, so sums of 1 or 2 truncate to 0 and the following pixel clock calculation divides by zero. Divide first and check the result.

Fixes b5c84a9edcd4 ("drm/bridge: add it6505 driver")

Signed-off-by Daniel Golle <[email protected]>

---
v6: no changes

v5: new patch

 drivers/gpu/drm/bridge/ite-it6505.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/gpu/drm/bridge/ite-it6505.c +2 -1

index 3806b0b96637..b7eb746c8f8e 100644
@@ -751,12 +751,13 @@ static void it6505_calc_video_info(struct it6505 *it6505)
 		sum += rddata;
 	}
 
+	sum /= 3;
+
 	if (sum == 0) {
 		DRM_DEV_DEBUG_DRIVER(dev, "calc video timing error");
 		return;
 	}
 
-	sum /= 3;
 	pclk = 13500 * 2048 / sum;
 	it6505->video_info.clock = pclk;
 	it6505->video_info.hdisplay = hdew;

Signature · 1 line

2.55.0

[email protected]

AI review

2026-07-25 03:20 UTC #11 lore raw

Re: [PATCH v6 06/13] drm/bridge: it6505: avoid division by zero in pixel clock calculation

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Unhandled hardware state when reading REG_AUDIO_INPUT_FREQ leads to division by zero and kernel panic.
- [Medium] Missing error handling for I2C reads leads to data corruption in video timing calculations.
- [Medium] Data race in it6505_calc_video_info corrupts pixel clock counter reading.
--

--- Patch [6]: [PATCH v6 06/13] drm/bridge: it6505: avoid division by zero in pixel clock calculation --- commit 00522edef86150c061773cb0103cee92136a7a79
Author: Daniel Golle <[email protected]>
Subject: drm/bridge: it6505: avoid division by zero in pixel clock calculation

This commit fixes a potential division by zero in the pixel clock computation. It moves the division of the counter sum to after the zero check to avoid truncation of small values. This prevents sums of 1 or 2 from being evaluated as 0 and bypassing the check.

drivers/gpu/drm/bridge/ite-it6505.c static void it6505_calc_video_info(struct it6505 *it6505) · line 751

Quoted 6 lines

drivers/gpu/drm/bridge/ite-it6505.c +0 -0

index 3806b0b966376..b7eb746c8f8e5 100644
@@ -751,12 +751,13 @@ static void it6505_calc_video_info(struct it6505 *it6505)
 		sum += rddata;

[Severity: Medium]
This is a pre-existing issue, but does this bitwise mask without checking for negative error codes cause data corruption?

If I2C reads fail and return -ENODEV, the negative return value is directly bitwise-masked and treated as a valid hardware reading.

For example, earlier in it6505_calc_video_info():

    htotal = it6505_read_word(it6505, REG_INPUT_HTOTAL) & 0x1FFF;

And similarly for the counter:

    rddata = it6505_read_word(it6505, REG_PCLK_COUNTER_VALUE) & 0xFFF;
    sum += rddata;

Could this lead to negative error codes being added to the counter sum, corrupting the subsequent pixel clock math?

[Severity: Medium]
This isn't a bug introduced by this patch, but is there a data race in it6505_calc_video_info() that corrupts the pixel clock counter reading?

This function performs a multi-step hardware sequence without holding any locks:

    it6505_set_bits(it6505, REG_DATA_CTRL0, ENABLE_PCLK_COUNTER,
                    ENABLE_PCLK_COUNTER);
    usleep_range(10000, 15000);
    it6505_set_bits(it6505, REG_DATA_CTRL0, ENABLE_PCLK_COUNTER, 0x00);
    rddata = ...

Since this can be executed concurrently by the threaded IRQ handler it6505_irq_scdt() and the debugfs read handler receive_timing_debugfs_show(), could a debugfs read occurring during the IRQ handling interleave register writes?

One thread might disable the counter prematurely while the other is sleeping, resulting in an abnormally small reading.

Is this race condition the root cause of the division by zero that this patch attempts to mitigate?

drivers/gpu/drm/bridge/ite-it6505.c static void it6505_calc_video_info(struct it6505 *it6505) · line 763

Quoted 13 lines

drivers/gpu/drm/bridge/ite-it6505.c +2 -1

 	}
 
+	sum /= 3;
+
 	if (sum == 0) {
 		DRM_DEV_DEBUG_DRIVER(dev, "calc video timing error");
 		return;
 	}
 
-	sum /= 3;
 	pclk = 13500 * 2048 / sum;
 	it6505->video_info.clock = pclk;
 	it6505->video_info.hdisplay = hdew;

[Severity: High]
This is a pre-existing issue not introduced by this patch, but can reading an unhandled hardware state lead to a kernel panic?

Looking at it6505_enable_audio():

    regbe = it6505_read(it6505, REG_AUDIO_INPUT_FREQ);
    DRM_DEV_DEBUG_DRIVER(dev, "regbe:0x%02x audio input fs: %d.%d kHz",
                         regbe, 6750 / regbe, (6750 % regbe) * 10 / regbe);

If the hardware register contains 0, for instance if the audio clock is not fully active or uninitialized, will the subsequent DRM_DEV_DEBUG_DRIVER macro trigger a division by zero when evaluating 6750 / regbe?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6

Daniel Golle

author

2026-07-25 02:59 UTC #12 lore raw

PATCH v6 7/13 drm/bridge: it6505: avoid division by zero in audio FS debug print

In reply to Daniel Golle

REG_AUDIO_INPUT_FREQ reads back 0 when no audio clock has been latched, and it6505_read() returns a negative errno on i2c failure. The debug print divides by the readback, and its arguments are evaluated even with debug output disabled. Only print when the value is usable as a divisor.

Fixes b5c84a9edcd4 ("drm/bridge: add it6505 driver")

Signed-off-by Daniel Golle <[email protected]>

---
v6: no changes

v5: new patch

 drivers/gpu/drm/bridge/ite-it6505.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

drivers/gpu/drm/bridge/ite-it6505.c +5 -2

index b7eb746c8f8e..04711c8f4a04 100644
@@ -1623,8 +1623,11 @@ static void it6505_enable_audio(struct it6505 *it6505)
 	it6505_set_bits(it6505, REG_AUDIO_SRC_CTRL, AUDIO_FIFO_RESET, 0x00);
 	it6505_set_bits(it6505, REG_RESET_CTRL, AUDIO_RESET, 0x00);
 	regbe = it6505_read(it6505, REG_AUDIO_INPUT_FREQ);
-	DRM_DEV_DEBUG_DRIVER(dev, "regbe:0x%02x audio input fs: %d.%d kHz",
-			     regbe, 6750 / regbe, (6750 % regbe) * 10 / regbe);
+	if (regbe > 0)
+		DRM_DEV_DEBUG_DRIVER(dev,
+				     "regbe:0x%02x audio input fs: %d.%d kHz",
+				     regbe, 6750 / regbe,
+				     (6750 % regbe) * 10 / regbe);
 	it6505_set_bits(it6505, REG_DATA_MUTE_CTRL, EN_AUD_MUTE, 0x00);
 }

Signature · 1 line

2.55.0

[email protected]

AI review

2026-07-25 03:16 UTC #13 lore raw

Re: [PATCH v6 07/13] drm/bridge: it6505: avoid division by zero in audio FS debug print

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Unhandled negative error code from it6505_read() is written back to REG_RESET_CTRL, inadvertently resetting the entire device.
- [High] Unhandled negative error codes from it6505_read_word() silently corrupt the video timing calculations. --

--- Patch [7]: [PATCH v6 07/13] drm/bridge: it6505: avoid division by zero in audio FS debug print --- commit fb85e4882527f59c0b49c06140294c57c71382ab
Author: Daniel Golle <[email protected]>

drm/bridge: it6505: avoid division by zero in audio FS debug print

This commit ensures the divisor is greater than zero before calculating and printing the audio input frequency in the debug output. This prevents a division by zero crash if the audio clock hasn't been latched and it6505_read() returns 0.

Quoted 7 lines

drivers/gpu/drm/bridge/ite-it6505.c +0 -0

index b7eb746c8f8e5..04711c8f4a041 100644
@@ -1623,8 +1623,11 @@ static void it6505_enable_audio(struct it6505 *it6505)
 	it6505_set_bits(it6505, REG_AUDIO_SRC_CTRL, AUDIO_FIFO_RESET, 0x00);
 	it6505_set_bits(it6505, REG_RESET_CTRL, AUDIO_RESET, 0x00);

drivers/gpu/drm/bridge/ite-it6505.c static void it6505_enable_audio(struct it6505 *it6505) · line 1632

Quoted 10 lines

drivers/gpu/drm/bridge/ite-it6505.c +5 -2

 	regbe = it6505_read(it6505, REG_AUDIO_INPUT_FREQ);
-	DRM_DEV_DEBUG_DRIVER(dev, "regbe:0x%02x audio input fs: %d.%d kHz",
-			     regbe, 6750 / regbe, (6750 % regbe) * 10 / regbe);
+	if (regbe > 0)
+		DRM_DEV_DEBUG_DRIVER(dev,
+				     "regbe:0x%02x audio input fs: %d.%d kHz",
+				     regbe, 6750 / regbe,
+				     (6750 % regbe) * 10 / regbe);
 	it6505_set_bits(it6505, REG_DATA_MUTE_CTRL, EN_AUD_MUTE, 0x00);
 }

[Severity: High]
This is a pre-existing issue, but I noticed another place where it6505_read() errors might cause problems. In it6505_audio_input(), if an I2C read fails, does this write the negative error code back to REG_RESET_CTRL?

drivers/gpu/drm/bridge/ite-it6505.c:it6505_audio_input() {
    ...
    reg05 = it6505_read(it6505, REG_RESET_CTRL);
    ...
    it6505_write(it6505, REG_RESET_CTRL, reg05);
    ...
}

If it6505_read() returns -19 (-ENODEV), the lower 8 bits (0xED) would set both ALL_LOGIC_RESET (BIT 2) and VIDEO_RESET (BIT 0). Could this unintentionally reset the entire hardware state machine?

[Severity: High]
This is also a pre-existing issue, but similar unhandled negative error codes seem to be present in it6505_calc_video_info():

drivers/gpu/drm/bridge/ite-it6505.c:it6505_calc_video_info() {
    ...
    htotal = it6505_read_word(it6505, REG_INPUT_HTOTAL) & 0x1FFF;
    hdes = it6505_read_word(it6505, REG_INPUT_HACTIVE_START) & 0x1FFF;
    ...
}

If it6505_read_word() returns a negative error code (e.g. -19), would the bitwise AND with 0x1FFF transform it into a large positive integer (e.g. 0x1FED)? Could these invalid values then corrupt the video timing state when accumulated into the pixel clock calculations?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7

Daniel Golle

author

2026-07-25 03:00 UTC #14 lore raw

PATCH v6 8/13 drm/bridge: it6505: guard against zero channel count in audio infoframe

In reply to Daniel Golle

it6505->audio.channel_count stays 0 until link training or a valid hw_params call has run, but it6505_enable_audio() can be reached before that from the audio-FIFO-error IRQ, making it6505_enable_audio_infoframe() index the 8-entry audio_info_ca[] table with -1. Bail out while the channel count is not yet known.

Also fix the debug print on the invalid-channel-count path, which logged the previously cached count instead of the rejected one.

Fixes b5c84a9edcd4 ("drm/bridge: add it6505 driver")

Signed-off-by Daniel Golle <[email protected]>

Reviewed-by Chen-Yu Tsai <[email protected]>

---
v6: no changes

v5: no changes

v4: no changes, collected Chen-Yu Tsai's Reviewed-by

v3: new patch

 drivers/gpu/drm/bridge/ite-it6505.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

drivers/gpu/drm/bridge/ite-it6505.c +4 -1

index 04711c8f4a04..f4ee5c81d3b8 100644
@@ -1577,6 +1577,9 @@ static void it6505_enable_audio_infoframe(struct it6505 *it6505)
 	struct device *dev = it6505->dev;
 	u8 audio_info_ca[] = { 0x00, 0x00, 0x01, 0x03, 0x07, 0x0B, 0x0F, 0x1F };
 
+	if (!it6505->audio.channel_count)
+		return;
+
 	DRM_DEV_DEBUG_DRIVER(dev, "infoframe channel_allocation:0x%02x",
 			     audio_info_ca[it6505->audio.channel_count - 1]);

@@ -3009,7 +3012,7 @@ static int __maybe_unused it6505_audio_setup_hw_params(struct it6505 *it6505,
 
 	if (params->cea.channels <= 1 || params->cea.channels > 8) {
 		DRM_DEV_DEBUG_DRIVER(dev, "channel number: %d not support",
-				     it6505->audio.channel_count);
+				     params->cea.channels);
 		return -EINVAL;
 	}

Signature · 1 line

2.55.0

Daniel Golle

author

2026-07-25 03:00 UTC #15 lore raw

PATCH v6 9/13 drm/bridge: it6505: hold endpoint OF node reference while parsing it

In reply to Daniel Golle

it6505_parse_dt() drops the reference to each endpoint node right after looking it up and then keeps reading its properties, racing with the node being freed under CONFIG_OF_DYNAMIC. Put the node only once parsing is done.

Fixes 380d920b582d ("drm/bridge: add it6505 driver to read data-lanes and link-frequencies from dt")

Signed-off-by Daniel Golle <[email protected]>

---
v6: no changes

v5: new patch

 drivers/gpu/drm/bridge/ite-it6505.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/gpu/drm/bridge/ite-it6505.c +2 -2

index f4ee5c81d3b8..0da8b40dd88a 100644
@@ -3366,7 +3366,6 @@ static void it6505_parse_dt(struct it6505 *it6505)
 	}
 
 	ep = of_graph_get_endpoint_by_regs(np, 1, 0);
-	of_node_put(ep);
 
 	if (ep) {
 		len = it6505_get_data_lanes_count(ep, 1, 4);

@@ -3379,13 +3378,13 @@ static void it6505_parse_dt(struct it6505 *it6505)
 			*max_lane_count = MAX_LANE_COUNT;
 			dev_err(dev, "error data-lanes, use default");
 		}
+		of_node_put(ep);
 	} else {
 		*max_lane_count = MAX_LANE_COUNT;
 		dev_err(dev, "error endpoint, use default");
 	}
 
 	ep = of_graph_get_endpoint_by_regs(np, 0, 0);
-	of_node_put(ep);
 
 	if (ep) {
 		len = of_property_read_variable_u64_array(ep,

@@ -3405,6 +3404,7 @@ static void it6505_parse_dt(struct it6505 *it6505)
 			dev_err(dev, "error link frequencies, use default");
 			*max_dpi_pixel_clock = DPI_PIXEL_CLK_MAX;
 		}
+		of_node_put(ep);
 	} else {
 		dev_err(dev, "error endpoint, use default");
 		*max_dpi_pixel_clock = DPI_PIXEL_CLK_MAX;

Signature · 1 line

2.55.0

[email protected]

AI review

2026-07-25 03:16 UTC #16 lore raw

Re: [PATCH v6 09/13] drm/bridge: it6505: hold endpoint OF node reference while parsing it

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Uninitialized stack variable link_frequencies is used if of_property_read_variable_u64_array() reads 0 elements. --

--- Patch [9]: [PATCH v6 09/13] drm/bridge: it6505: hold endpoint OF node reference while parsing it --- commit 635833fd1fe628586cc12b54a91fac7c8f243c4c
Author: Daniel Golle <[email protected]>

drm/bridge: it6505: hold endpoint OF node reference while parsing it

This commit delays dropping the reference to endpoint nodes in it6505_parse_dt() until after their properties have been parsed. This prevents a race condition where the node could be freed while its properties are still being read when CONFIG_OF_DYNAMIC is enabled.

Quoted 3 lines

drivers/gpu/drm/bridge/ite-it6505.c +0 -0

[ ... ]

Quoted 1 line

drivers/gpu/drm/bridge/ite-it6505.c +0 -0
@@ -3379,13 +3378,13 @@ static void it6505_parse_dt(struct it6505 *it6505)

[ ... ]

drivers/gpu/drm/bridge/ite-it6505.c static void it6505_parse_dt(struct it6505 *it6505) · line 3390

Quoted 5 lines

drivers/gpu/drm/bridge/ite-it6505.c +0 -1

 	ep = of_graph_get_endpoint_by_regs(np, 0, 0);
-	of_node_put(ep);
 
 	if (ep) {
 		len = of_property_read_variable_u64_array(ep,

[Severity: High]
This is a pre-existing issue, but can this lead to using uninitialized stack memory?

If the link-frequencies property exists in the device tree but is empty, of_property_read_variable_u64_array() (called with sz_min set to 0) will return 0 and leave the link_frequencies stack variable unmodified.

Since the subsequent check if (len >= 0) evaluates to true, won't the code proceed to pass the uninitialized link_frequencies variable to do_div() and use it to calculate the maximum DPI pixel clock?

Quoted 8 lines

drivers/gpu/drm/bridge/ite-it6505.c +1 -0

@@ -3405,6 +3404,7 @@ static void it6505_parse_dt(struct it6505 *it6505)
 			dev_err(dev, "error link frequencies, use default");
 			*max_dpi_pixel_clock = DPI_PIXEL_CLK_MAX;
 		}
+		of_node_put(ep);
 	} else {
 		dev_err(dev, "error endpoint, use default");
 		*max_dpi_pixel_clock = DPI_PIXEL_CLK_MAX;

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9

Daniel Golle

author

2026-07-25 03:00 UTC #17 lore raw

PATCH v6 10/13 drm/bridge: it6505: reject a too short link-frequencies property

In reply to Daniel Golle

of_property_read_variable_u64_array() is asked for a minimum array size of zero, so a link-frequencies property shorter than one u64 -- a 32-bit value written by mistake, say -- returns 0 without storing anything. it6505_parse_dt() reads that as success and derives max_dpi_pixel_clock from an uninitialised stack variable. Ask for one element so a short property lands in the existing error path.

Fixes 380d920b582d ("drm/bridge: add it6505 driver to read data-lanes and link-frequencies from dt")

Signed-off-by Daniel Golle <[email protected]>

---
v6: new patch

 drivers/gpu/drm/bridge/ite-it6505.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/gpu/drm/bridge/ite-it6505.c +1 -1

index 0da8b40dd88a..1678ec5b3ba7 100644
@@ -3389,7 +3389,7 @@ static void it6505_parse_dt(struct it6505 *it6505)
 	if (ep) {
 		len = of_property_read_variable_u64_array(ep,
 							  "link-frequencies",
-							  &link_frequencies, 0,
+							  &link_frequencies, 1,
 							  1);
 		if (len >= 0) {
 			do_div(link_frequencies, 1000);

Signature · 1 line

2.55.0

Daniel Golle

author

2026-07-25 03:00 UTC #18 lore raw

PATCH v6 11/13 drm/bridge: it6505: don't write an error code back to the reset register

In reply to Daniel Golle

it6505_audio_input() saves REG_RESET_CTRL, pulses AUDIO_RESET and writes the saved value back. it6505_read() returns a negative errno on i2c failure, which regmap truncates to eight bits on the way out:
-EIO turns into 0xfb, asserting ALL_LOGIC_RESET, VIDEO_RESET, AUX_RESET and HDCP_RESET. A failed REG_AUDIO_INPUT_FREQ read is just as wrong, as any negative value passes the != 0xFF test and reports an audio clock that was never sampled. Bail out before touching the register and only report audio input for a usable readback.

Fixes b5c84a9edcd4 ("drm/bridge: add it6505 driver")

Signed-off-by Daniel Golle <[email protected]>

---
v6: new patch

 drivers/gpu/drm/bridge/ite-it6505.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

drivers/gpu/drm/bridge/ite-it6505.c +4 -1

index 1678ec5b3ba7..aeaa7b5ecf5a 100644
@@ -1516,12 +1516,15 @@ static bool it6505_audio_input(struct it6505 *it6505)
 	int reg05, regbe;
 
 	reg05 = it6505_read(it6505, REG_RESET_CTRL);
+	if (reg05 < 0)
+		return false;
+
 	it6505_set_bits(it6505, REG_RESET_CTRL, AUDIO_RESET, 0x00);
 	usleep_range(3000, 4000);
 	regbe = it6505_read(it6505, REG_AUDIO_INPUT_FREQ);
 	it6505_write(it6505, REG_RESET_CTRL, reg05);
 
-	return regbe != 0xFF;
+	return regbe > 0 && regbe != 0xFF;
 }
 
 static void it6505_setup_audio_channel_status(struct it6505 *it6505)

Signature · 1 line

2.55.0

[email protected]

AI review

2026-07-25 03:19 UTC #19 lore raw

Re: [PATCH v6 11/13] drm/bridge: it6505: don't write an error code back to the reset register

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing error check for it6505_read() inside it6505_aux_i2c_wait() causes I2C read failures to be silently truncated and falsely treated as successful AUX operations, populating caller buffers with error codes masquerading as valid data.
- [High] Blind restoration of the entire REG_RESET_CTRL register in it6505_audio_input() clobbers concurrent modifications made by other threads or workqueues.
--

--- Patch [11]: [PATCH v6 11/13] drm/bridge: it6505: don't write an error code back to the reset register --- commit 6cdd627967e5d418a885e4cc00b3b1ef771660fc
Author: Daniel Golle <[email protected]>

drm/bridge: it6505: don't write an error code back to the reset register

This commit addresses an issue in it6505_audio_input() where a negative error code from it6505_read() could be written back to the reset register. It adds a check to return early if reading REG_RESET_CTRL fails, and also ensures the audio input frequency readback is strictly positive.

Quoted 5 lines

drivers/gpu/drm/bridge/ite-it6505.c +0 -0
index 1678ec5b3ba70..aeaa7b5ecf5aa 100644

drivers/gpu/drm/bridge/ite-it6505.c static bool it6505_audio_input(struct it6505 *it6505) · line 1525

Quoted 10 lines

drivers/gpu/drm/bridge/ite-it6505.c +3 -0

@@ -1516,12 +1516,15 @@ static bool it6505_audio_input(struct it6505 *it6505)
 	int reg05, regbe;
 
 	reg05 = it6505_read(it6505, REG_RESET_CTRL);
+	if (reg05 < 0)
+		return false;
+
 	it6505_set_bits(it6505, REG_RESET_CTRL, AUDIO_RESET, 0x00);
 	usleep_range(3000, 4000);
 	regbe = it6505_read(it6505, REG_AUDIO_INPUT_FREQ);
 	it6505_write(it6505, REG_RESET_CTRL, reg05);

[Severity: High]
This is a pre-existing issue, but does this blind write back of the cached reg05 value clobber concurrent modifications?

If this is called from the threaded IRQ handler or link_works, it can run concurrently with the hdcp_work delayed workqueue. If hdcp_work clears HDCP_RESET during the 3000-4000us sleep window, that concurrent change will be permanently overwritten and lost when the stale reg05 value is restored here.

drivers/gpu/drm/bridge/ite-it6505.c static bool it6505_audio_input(struct it6505 *it6505) · line 1530

Quoted 5 lines

drivers/gpu/drm/bridge/ite-it6505.c +1 -1

-	return regbe != 0xFF;
+	return regbe > 0 && regbe != 0xFF;
 }
 
 static void it6505_setup_audio_channel_status(struct it6505 *it6505)

While reviewing this fix for missing error checks, I noticed another similar pre-existing issue in the same file:

[Severity: High]
This isn't a bug introduced by this patch, but does the lack of an error check for it6505_read() inside it6505_aux_i2c_wait() cause I2C read failures to be treated as successful operations?

drivers/gpu/drm/bridge/ite-it6505.c:it6505_aux_i2c_wait() {
    ...
    if (it6505_read(it6505, REG_AUX_USER_CTRL) & AUX_EVENT)
        break;
    ...
    *reply = it6505_read(it6505, REG_AUX_USER_REPLY) >> 4;
    ...
}

If it6505_read(it6505, REG_AUX_USER_CTRL) fails, it returns a negative errno like -EIO (-5 = 0xFFFFFFFB). The bitwise check & AUX_EVENT (0x10) will incorrectly evaluate to true, breaking the wait loop.

Then, if the read of REG_AUX_USER_REPLY also fails, the negative return is right-shifted and truncated to 0xFF, which avoids error assignment and returns success.

This eventually leads to it6505_aux_i2c_readb() reading garbage data into the output buffer:

drivers/gpu/drm/bridge/ite-it6505.c:it6505_aux_i2c_readb() {
    ...
    buf[i] = it6505_read(it6505, REG_AUX_USER_RXB(0 + i));
    ...
}

Could this silently propagate hardware error codes to the DRM layer as valid data?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11

Daniel Golle

author

2026-07-25 03:01 UTC #20 lore raw

PATCH v6 12/13 drm/bridge: it6505: Add audio support

In reply to Daniel Golle

From: Jiaxin Yu <[email protected]>

Add audio support for it6505 by bridging to the hdmi-codec: register an "hdmi-audio-codec" platform device from probe and wire up the previously unused audio helpers via hdmi_codec_ops. This unblocks the mt8186-mt6366 sound card which references it6505 as the I2S3 codec.

Audio starts out muted and is only enabled once the stream is unmuted, since some DP-to-HDMI dongles get into a bad state if the InfoFrame is sent without audio data. Enable/disable can race between the FIFO-error IRQ, the delayed enable work, the hdmi-codec ops and the HPD-low path, so the register sequences, the mute state and the cached stream parameters are serialised with a new audio_lock mutex. plugged_cb/codec_dev updates take mode_lock, the lock it6505_detect() holds when calling back into the codec.

Signed-off-by Jiaxin Yu <[email protected]>

Link https://lore.kernel.org/all/[email protected]/ open here

Signed-off-by Daniel Golle <[email protected]>

---
v6: no changes

v5: update plugged_cb/codec_dev under mode_lock

v4:
* start out muted
* update mute state and cached parameters under audio_lock; the FIFO-error IRQ probes and enables audio in one locked section
* unregister the codec device only after quiesce on remove

v3: serialise enable/disable with audio_lock; track mute state so
the FIFO-error IRQ cannot undo a mute

v2:
* drive enable/disable from .mute_stream (hdmi_codec_ops lost .trigger), use it6505->dev
* keep and unregister the codec platform_device on remove, cancel delayed_audio on shutdown and remove, disable audio when muting

v1: respin of Jiaxin Yu's v3 on current -next

 drivers/gpu/drm/bridge/ite-it6505.c | 163 +++++++++++++++++++++++-----
 1 file changed, 134 insertions(+), 29 deletions(-)

drivers/gpu/drm/bridge/ite-it6505.c +134 ~~-29~~

index aeaa7b5ecf5a..c560f1258f6d 100644
@@ -407,6 +407,7 @@ struct it6505_audio_data {
 	u8 i2s_data_delay;
 	u8 i2s_ws_channel;
 	u8 i2s_data_sequence;
+	bool mute;
 };
 
 struct it6505_audio_sample_rate_map {

@@ -439,6 +440,7 @@ struct it6505 {
 	struct mutex extcon_lock;
 	struct mutex mode_lock; /* used to bridge_detect */
 	struct mutex aux_lock; /* used to aux data transfers */
+	struct mutex audio_lock; /* serializes audio enable/disable */
 	struct regmap *regmap;
 	struct drm_display_mode source_output_mode;
 	struct drm_display_mode video_info;

@@ -477,6 +479,7 @@ struct it6505 {
 	bool enable_enhanced_frame;
 	hdmi_codec_plugged_cb plugged_cb;
 	struct device *codec_dev;
+	struct platform_device *audio_pdev;
 	struct delayed_work delayed_audio;
 	struct it6505_audio_data audio;
 	struct dentry *debugfs;

@@ -1599,7 +1602,7 @@ static void it6505_enable_audio_infoframe(struct it6505 *it6505)
 			EN_AUD_CTRL_PKT);
 }
 
-static void it6505_disable_audio(struct it6505 *it6505)
+static void __it6505_disable_audio(struct it6505 *it6505)
 {
 	it6505_set_bits(it6505, REG_DATA_MUTE_CTRL, EN_AUD_MUTE, EN_AUD_MUTE);
 	it6505_set_bits(it6505, REG_AUDIO_SRC_CTRL, M_AUDIO_I2S_EN, 0x00);

@@ -1607,13 +1610,21 @@ static void it6505_disable_audio(struct it6505 *it6505)
 	it6505_set_bits(it6505, REG_RESET_CTRL, AUDIO_RESET, AUDIO_RESET);
 }
 
-static void it6505_enable_audio(struct it6505 *it6505)
+static void it6505_disable_audio(struct it6505 *it6505)
+{
+	mutex_lock(&it6505->audio_lock);
+	__it6505_disable_audio(it6505);
+	mutex_unlock(&it6505->audio_lock);
+}
+
+static void __it6505_enable_audio(struct it6505 *it6505)
 {
 	struct device *dev = it6505->dev;
 	int regbe;
 
 	DRM_DEV_DEBUG_DRIVER(dev, "start");
-	it6505_disable_audio(it6505);
+
+	__it6505_disable_audio(it6505);
 
 	it6505_setup_audio_channel_status(it6505);
 	it6505_setup_audio_format(it6505);

@@ -1637,6 +1648,14 @@ static void it6505_enable_audio(struct it6505 *it6505)
 	it6505_set_bits(it6505, REG_DATA_MUTE_CTRL, EN_AUD_MUTE, 0x00);
 }
 
+static void it6505_enable_audio(struct it6505 *it6505)
+{
+	mutex_lock(&it6505->audio_lock);
+	if (!it6505->audio.mute)
+		__it6505_enable_audio(it6505);
+	mutex_unlock(&it6505->audio_lock);
+}
+
 static bool it6505_use_step_train_check(struct it6505 *it6505)
 {
 	if (it6505->link.revision >= 0x12)

@@ -2331,19 +2350,12 @@ static void it6505_stop_link_train(struct it6505 *it6505)
 
 static void it6505_link_train_ok(struct it6505 *it6505)
 {
-	struct device *dev = it6505->dev;
-
 	it6505->link_state = LINK_OK;
 	/* disalbe mute enable avi info frame */
 	it6505_set_bits(it6505, REG_DATA_MUTE_CTRL, EN_VID_MUTE, 0x00);
 	it6505_set_bits(it6505, REG_INFOFRAME_CTRL,
 			EN_VID_CTRL_PKT, EN_VID_CTRL_PKT);
 
-	if (it6505_audio_input(it6505)) {
-		DRM_DEV_DEBUG_DRIVER(dev, "Enable audio!");
-		it6505_enable_audio(it6505);
-	}
-
 	if (it6505->hdcp_desired)
 		it6505_start_hdcp(it6505);
 }

@@ -2635,8 +2647,10 @@ static void it6505_irq_audio_fifo_error(struct it6505 *it6505)
 
 	DRM_DEV_DEBUG_DRIVER(dev, "audio fifo error Interrupt");
 
-	if (it6505_audio_input(it6505))
-		it6505_enable_audio(it6505);
+	mutex_lock(&it6505->audio_lock);
+	if (!it6505->audio.mute && it6505_audio_input(it6505))
+		__it6505_enable_audio(it6505);
+	mutex_unlock(&it6505->audio_lock);
 }
 
 static void it6505_irq_link_train_fail(struct it6505 *it6505)

@@ -2985,7 +2999,7 @@ static void it6505_remove_notifier_module(struct it6505 *it6505)
 	flush_work(&it6505->extcon_wq);
 }
 
-static void __maybe_unused it6505_delayed_audio(struct work_struct *work)
+static void it6505_delayed_audio(struct work_struct *work)
 {
 	struct it6505 *it6505 = container_of(work, struct it6505,
 					     delayed_audio.work);

@@ -2999,11 +3013,12 @@ static void __maybe_unused it6505_delayed_audio(struct work_struct *work)
 		it6505_enable_audio(it6505);
 }
 
-static int __maybe_unused it6505_audio_setup_hw_params(struct it6505 *it6505,
-						       struct hdmi_codec_params
-						       *params)
+static int it6505_audio_setup_hw_params(struct it6505 *it6505,
+					struct hdmi_codec_params
+					*params)
 {
 	struct device *dev = it6505->dev;
+	u8 word_length;
 	int i = 0;
 
 	DRM_DEV_DEBUG_DRIVER(dev, "%s %d Hz, %d bit, %d channels\n", __func__,

@@ -3019,8 +3034,6 @@ static int __maybe_unused it6505_audio_setup_hw_params(struct it6505 *it6505,
 		return -EINVAL;
 	}
 
-	it6505->audio.channel_count = params->cea.channels;
-
 	while (i < ARRAY_SIZE(audio_sample_rate_map) &&
 	       params->sample_rate !=
 		       audio_sample_rate_map[i].sample_rate_value) {

@@ -3031,21 +3044,20 @@ static int __maybe_unused it6505_audio_setup_hw_params(struct it6505 *it6505,
 				     params->sample_rate);
 		return -EINVAL;
 	}
-	it6505->audio.sample_rate = audio_sample_rate_map[i].rate;
 
 	switch (params->sample_width) {
 	case 16:
-		it6505->audio.word_length = WORD_LENGTH_16BIT;
+		word_length = WORD_LENGTH_16BIT;
 		break;
 	case 18:
-		it6505->audio.word_length = WORD_LENGTH_18BIT;
+		word_length = WORD_LENGTH_18BIT;
 		break;
 	case 20:
-		it6505->audio.word_length = WORD_LENGTH_20BIT;
+		word_length = WORD_LENGTH_20BIT;
 		break;
 	case 24:
 	case 32:
-		it6505->audio.word_length = WORD_LENGTH_24BIT;
+		word_length = WORD_LENGTH_24BIT;
 		break;
 	default:
 		DRM_DEV_DEBUG_DRIVER(dev, "wordlength: %d bit not support",

@@ -3053,27 +3065,111 @@ static int __maybe_unused it6505_audio_setup_hw_params(struct it6505 *it6505,
 		return -EINVAL;
 	}
 
+	mutex_lock(&it6505->audio_lock);
+	it6505->audio.channel_count = params->cea.channels;
+	it6505->audio.sample_rate = audio_sample_rate_map[i].rate;
+	it6505->audio.word_length = word_length;
+	mutex_unlock(&it6505->audio_lock);
+
 	return 0;
 }
 
-static void __maybe_unused it6505_audio_shutdown(struct device *dev, void *data)
+static void it6505_audio_shutdown(struct device *dev, void *data)
 {
 	struct it6505 *it6505 = dev_get_drvdata(dev);
 
+	mutex_lock(&it6505->audio_lock);
+	it6505->audio.mute = true;
 	if (it6505->powered)
-		it6505_disable_audio(it6505);
+		__it6505_disable_audio(it6505);
+	mutex_unlock(&it6505->audio_lock);
+	cancel_delayed_work_sync(&it6505->delayed_audio);
+}
+
+static int it6505_audio_hw_params(struct device *dev, void *data,
+				  struct hdmi_codec_daifmt *daifmt,
+				  struct hdmi_codec_params *params)
+{
+	struct it6505 *it6505 = dev_get_drvdata(dev);
+
+	return it6505_audio_setup_hw_params(it6505, params);
 }
 
-static int __maybe_unused it6505_audio_hook_plugged_cb(struct device *dev,
-						       void *data,
-						       hdmi_codec_plugged_cb fn,
-						       struct device *codec_dev)
+static int it6505_audio_mute(struct device *dev, void *data,
+			     bool enable, int direction)
+{
+	struct it6505 *it6505 = dev_get_drvdata(dev);
+
+	DRM_DEV_DEBUG_DRIVER(dev, "mute: %d", enable);
+
+	/*
+	 * Delay enabling audio until the stream is unmuted; InfoFrames
+	 * without audio data upset some DP-to-HDMI dongles.
+	 */
+	if (enable) {
+		mutex_lock(&it6505->audio_lock);
+		it6505->audio.mute = true;
+		if (it6505->powered)
+			__it6505_disable_audio(it6505);
+		mutex_unlock(&it6505->audio_lock);
+		cancel_delayed_work_sync(&it6505->delayed_audio);
+	} else {
+		mutex_lock(&it6505->audio_lock);
+		it6505->audio.mute = false;
+		mutex_unlock(&it6505->audio_lock);
+		queue_delayed_work(system_wq, &it6505->delayed_audio,
+				   msecs_to_jiffies(180));
+	}
+
+	return 0;
+}
+
+static int it6505_audio_hook_plugged_cb(struct device *dev,
+					void *data,
+					hdmi_codec_plugged_cb fn,
+					struct device *codec_dev)
 {
 	struct it6505 *it6505 = data;
 
+	mutex_lock(&it6505->mode_lock);
 	it6505->plugged_cb = fn;
 	it6505->codec_dev = codec_dev;
 	it6505_plugged_status_to_codec(it6505);
+	mutex_unlock(&it6505->mode_lock);
+
+	return 0;
+}
+
+static const struct hdmi_codec_ops it6505_audio_codec_ops = {
+	.hw_params = it6505_audio_hw_params,
+	.mute_stream = it6505_audio_mute,
+	.audio_shutdown = it6505_audio_shutdown,
+	.hook_plugged_cb = it6505_audio_hook_plugged_cb,
+};
+
+static int it6505_register_audio_driver(struct device *dev)
+{
+	struct it6505 *it6505 = dev_get_drvdata(dev);
+	struct hdmi_codec_pdata codec_data = {
+		.ops = &it6505_audio_codec_ops,
+		.max_i2s_channels = 8,
+		.i2s = 1,
+		.no_capture_mute = 1,
+		.data = it6505,
+	};
+	struct platform_device *pdev;
+
+	it6505->audio.mute = true;
+	INIT_DELAYED_WORK(&it6505->delayed_audio, it6505_delayed_audio);
+
+	pdev = platform_device_register_data(dev, HDMI_CODEC_DRV_NAME,
+					     PLATFORM_DEVID_AUTO, &codec_data,
+					     sizeof(codec_data));
+	if (IS_ERR(pdev))
+		return PTR_ERR(pdev);
+
+	it6505->audio_pdev = pdev;
+	DRM_DEV_DEBUG_DRIVER(dev, "bound to %s", HDMI_CODEC_DRV_NAME);
 
 	return 0;
 }

@@ -3589,6 +3685,7 @@ static int it6505_i2c_probe(struct i2c_client *client)
 	mutex_init(&it6505->extcon_lock);
 	mutex_init(&it6505->mode_lock);
 	mutex_init(&it6505->aux_lock);
+	mutex_init(&it6505->audio_lock);
 
 	it6505->bridge.of_node = client->dev.of_node;
 	it6505->connector_status = connector_status_disconnected;

@@ -3639,6 +3736,12 @@ static int it6505_i2c_probe(struct i2c_client *client)
 		return err;
 	}
 
+	err = it6505_register_audio_driver(dev);
+	if (err < 0) {
+		dev_err(dev, "Failed to register audio driver: %d", err);
+		return err;
+	}
+
 	INIT_WORK(&it6505->link_works, it6505_link_training_work);
 	INIT_WORK(&it6505->hdcp_wait_ksv_list, it6505_hdcp_wait_ksv_list);
 	INIT_DELAYED_WORK(&it6505->hdcp_work, it6505_hdcp_work);

@@ -3680,6 +3783,8 @@ static void it6505_i2c_remove(struct i2c_client *client)
 	cancel_work_sync(&it6505->hdcp_wait_ksv_list);
 	cancel_delayed_work_sync(&it6505->hdcp_work);
 	cancel_work_sync(&it6505->extcon_wq);
+	platform_device_unregister(it6505->audio_pdev);
+	cancel_delayed_work_sync(&it6505->delayed_audio);
 	if (it6505->extcon_state)
 		pm_runtime_put_sync(&client->dev);
 	pm_runtime_disable(&client->dev);

Signature · 1 line

2.55.0

[email protected]

AI review

2026-07-25 05:25 UTC #21 lore raw

Re: [PATCH v6 12/13] drm/bridge: it6505: Add audio support

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Missing PM runtime reference in audio callbacks leads to TOCTOU race with device power-off. --

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing PM runtime reference in audio callbacks leads to TOCTOU race with device power-off. --

--- Patch [12]: [PATCH v6 12/13] drm/bridge: it6505: Add audio support --- commit 10341e11c958ca412de2f96980c0f7ee9f5854c5
Author: Jiaxin Yu <[email protected]>

drm/bridge: it6505: Add audio support

Link https://lore.kernel.org/all/[email protected]/ open here

Quoted 4 lines

drivers/gpu/drm/bridge/ite-it6505.c +0 -0
index aeaa7b5ecf5aa..c560f1258f6d7 100644

[ ... ]

Quoted 13 lines

drivers/gpu/drm/bridge/ite-it6505.c +7 -2

-static void __maybe_unused it6505_audio_shutdown(struct device *dev, void *data)
+static void it6505_audio_shutdown(struct device *dev, void *data)
 {
 	struct it6505 *it6505 = dev_get_drvdata(dev);
 
+	mutex_lock(&it6505->audio_lock);
+	it6505->audio.mute = true;
 	if (it6505->powered)
-		it6505_disable_audio(it6505);
+		__it6505_disable_audio(it6505);
+	mutex_unlock(&it6505->audio_lock);
+	cancel_delayed_work_sync(&it6505->delayed_audio);
+}

[ ... ]

Quoted 18 lines

drivers/gpu/drm/bridge/ite-it6505.c +18 -0

+static int it6505_audio_mute(struct device *dev, void *data,
+			     bool enable, int direction)
+{
+	struct it6505 *it6505 = dev_get_drvdata(dev);
+
+	DRM_DEV_DEBUG_DRIVER(dev, "mute: %d", enable);
+
+	/*
+	 * Delay enabling audio until the stream is unmuted; InfoFrames
+	 * without audio data upset some DP-to-HDMI dongles.
+	 */
+	if (enable) {
+		mutex_lock(&it6505->audio_lock);
+		it6505->audio.mute = true;
+		if (it6505->powered)
+			__it6505_disable_audio(it6505);
+		mutex_unlock(&it6505->audio_lock);
+		cancel_delayed_work_sync(&it6505->delayed_audio);

drivers/gpu/drm/bridge/ite-it6505.c static int __maybe_unused it6505_audio_setup_hw_params(struct it6505 *it6505, · line 3125

Quoted 10 lines

drivers/gpu/drm/bridge/ite-it6505.c +10 -0

+	} else {
+		mutex_lock(&it6505->audio_lock);
+		it6505->audio.mute = false;
+		mutex_unlock(&it6505->audio_lock);
+		queue_delayed_work(system_wq, &it6505->delayed_audio,
+				   msecs_to_jiffies(180));
+	}
+
+	return 0;
+}

[Severity: High]
Does this code race with device power-off?

If the audio stream is unmuted via ALSA, it6505_audio_mute() queues it6505->delayed_audio to run after 180ms.

If a user unplugs the DP cable during this delay, it6505_extcon_work() calls pm_runtime_put_sync() on the parent device, invoking the runtime suspend callback it6505_poweroff().

it6505_poweroff() disables regulators and asserts the reset GPIO before clearing the it6505->powered flag at the very end of the function. It does not cancel the delayed work.

If it6505_delayed_audio() (or a concurrent it6505_audio_mute() / it6505_audio_shutdown()) checks it6505->powered before it6505_poweroff() updates the flag, they could proceed to issue I2C writes while the device's power is physically cut. This can result in I2C bus lockups since unpowered chips may pull SDA/SCL lines low.

Should these functions use a PM runtime reference (e.g., via pm_runtime_get_if_active()) to safely access the hardware?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=12

Daniel Golle

author

2026-07-25 03:01 UTC #22 lore raw

PATCH v6 13/13 drm/bridge: it6505: Don't reject audio hw_params without an encoder

In reply to Daniel Golle

it6505_audio_setup_hw_params() returns -ENODEV when no encoder is attached. With it6505 registering an hdmi-audio-codec this runs for every stream on the I2S DAI, and on mt8186-corsola, where the rt1019 speaker amplifier shares I2S3, the error tears down the whole DPCM backend and breaks speaker playback whenever no display is attached.

The function only caches stream parameters, which needs no encoder. Drop the check, and apply the audio defaults once at probe instead of in it6505_variable_config(), which would clobber the cached parameters again on every hotplug. Actual audio output remains gated by it6505->powered.

Fixes b5c84a9edcd4 ("drm/bridge: add it6505 driver")

Signed-off-by Daniel Golle <[email protected]>

---
v6: no changes

v5: no changes

v4: apply audio defaults at probe time instead of in

    it6505_variable_config()

v3: no changes

v2: drop the encoder check entirely instead of returning 0 early

 drivers/gpu/drm/bridge/ite-it6505.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

drivers/gpu/drm/bridge/ite-it6505.c +10 ~~-13~~

index c560f1258f6d..0cb16be45d93 100644
@@ -1371,16 +1371,6 @@ static void it6505_variable_config(struct it6505 *it6505)
 	it6505->link_state = LINK_IDLE;
 	it6505->hdcp_desired = HDCP_DESIRED;
 	it6505->auto_train_retry = AUTO_TRAIN_RETRY;
-	it6505->audio.select = AUDIO_SELECT;
-	it6505->audio.sample_rate = AUDIO_SAMPLE_RATE;
-	it6505->audio.channel_count = AUDIO_CHANNEL_COUNT;
-	it6505->audio.type = AUDIO_TYPE;
-	it6505->audio.i2s_input_format = I2S_INPUT_FORMAT;
-	it6505->audio.i2s_justified = I2S_JUSTIFIED;
-	it6505->audio.i2s_data_delay = I2S_DATA_DELAY;
-	it6505->audio.i2s_ws_channel = I2S_WS_CHANNEL;
-	it6505->audio.i2s_data_sequence = I2S_DATA_SEQUENCE;
-	it6505->audio.word_length = AUDIO_WORD_LENGTH;
 	memset(it6505->sha1_input, 0, sizeof(it6505->sha1_input));
 	memset(it6505->bksvs, 0, sizeof(it6505->bksvs));
 }

@@ -3025,9 +3015,6 @@ static int it6505_audio_setup_hw_params(struct it6505 *it6505,
 			     params->sample_rate, params->sample_width,
 			     params->cea.channels);
 
-	if (!it6505->bridge.encoder)
-		return -ENODEV;
-
 	if (params->cea.channels <= 1 || params->cea.channels > 8) {
 		DRM_DEV_DEBUG_DRIVER(dev, "channel number: %d not support",
 				     params->cea.channels);

@@ -3159,6 +3146,16 @@ static int it6505_register_audio_driver(struct device *dev)
 	};
 	struct platform_device *pdev;
 
+	it6505->audio.select = AUDIO_SELECT;
+	it6505->audio.sample_rate = AUDIO_SAMPLE_RATE;
+	it6505->audio.channel_count = AUDIO_CHANNEL_COUNT;
+	it6505->audio.type = AUDIO_TYPE;
+	it6505->audio.i2s_input_format = I2S_INPUT_FORMAT;
+	it6505->audio.i2s_justified = I2S_JUSTIFIED;
+	it6505->audio.i2s_data_delay = I2S_DATA_DELAY;
+	it6505->audio.i2s_ws_channel = I2S_WS_CHANNEL;
+	it6505->audio.i2s_data_sequence = I2S_DATA_SEQUENCE;
+	it6505->audio.word_length = AUDIO_WORD_LENGTH;
 	it6505->audio.mute = true;
 	INIT_DELAYED_WORK(&it6505->delayed_audio, it6505_delayed_audio);

Signature · 1 line

2.55.0

AngeloGioacchino Del Regno

maintainer Reviewed-by Tested-by on the v6 series

2026-07-27 11:34 UTC #23 lore raw

In reply to Daniel Golle

Quoted 17 lines · Daniel Golle

Revive HDMI/DP audio support for the it6505, originally posted by Jiaxin Yu [1], rebased onto current -next with the review feedback
addressed: &client->dev is gone (the driver gained it6505->dev) and "#sound-dai-cells" has since been added by commit 325af1bef5b9 ("dt-bindings: display: bridge: it6505: Add #sound-dai-cells").
Patches 1-11 fix pre-existing bugs found during review of earlier versions of this series; they carry Fixes: tags and are ordered first so stable trees can pick them.
Patch 12 wires up the it6505 audio helpers via hdmi_codec_ops, unblocking the mt8186-mt6366 sound card which references it6505 as the I2S3 codec. Patch 13 makes hw_params succeed with no display attached, needed when the I2S bus is shared with another codec as on MT8186 "steelix" Chromebooks, where the speaker path must keep working regardless of the it6505 link state.

Quoted 4 lines

Tested on a MT8186 (google,steelix) Chromebook: the sound card comes up, speakers, microphone and headset jack work. HDMI/DP audio output itself could not be tested for lack of a suitable adapter cable.

Whole series is

Reviewed-by AngeloGioacchino Del Regno <[email protected]>

Tested-by AngeloGioachcino Del Regno <[email protected]> # Steelix Chromebook

Cheers,
Angelo

Quoted 1 line

[1] https://lore.kernel.org/all/[email protected]/

Daniel Golle

author

2026-08-31 16:34 UTC #24 lore raw

Quoted 23 lines · AngeloGioacchino Del Regno

Quoted 17 lines · Daniel Golle
Revive HDMI/DP audio support for the it6505, originally posted by Jiaxin Yu [1], rebased onto current -next with the review feedback
addressed: &client->dev is gone (the driver gained it6505->dev) and "#sound-dai-cells" has since been added by commit 325af1bef5b9 ("dt-bindings: display: bridge: it6505: Add #sound-dai-cells").
Patches 1-11 fix pre-existing bugs found during review of earlier versions of this series; they carry Fixes: tags and are ordered first so stable trees can pick them.
Patch 12 wires up the it6505 audio helpers via hdmi_codec_ops, unblocking the mt8186-mt6366 sound card which references it6505 as the I2S3 codec. Patch 13 makes hw_params succeed with no display attached, needed when the I2S bus is shared with another codec as on MT8186 "steelix" Chromebooks, where the speaker path must keep working regardless of the it6505 link state.
Quoted 4 lines
Tested on a MT8186 (google,steelix) Chromebook: the sound card comes up, speakers, microphone and headset jack work. HDMI/DP audio output itself could not be tested for lack of a suitable adapter cable.

Quoted 5 lines

Whole series is
Reviewed-by AngeloGioacchino Del Regno <[email protected]>
Tested-by AngeloGioachcino Del Regno
<[email protected]> # Steelix Chromebook

Any chance to get this merged in the new cycle now?