From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/bridge: Implement generic USB Type-C DP HPD bridge Date: Mon, 25 May 2026 21:00:52 +1000 Message-ID: In-Reply-To: <20260521032854.103-2-kernel@airkyi.com> References: <20260521032854.103-1-kernel@airkyi.com> <20260521032854.103-2-kernel@airkyi.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review This patch adds a new module that monitors the Type-C bus for altmode devices with DP SVID and automatically creates an HPD bridge for them. **Issue 1: No error handling for bridge registration failure** ```c if (is_typec_port_altmode(&alt->dev) && alt->svid == USB_TYPEC_DP_SID) drm_dp_hpd_bridge_register(alt->dev.parent->parent, to_of_node(alt->dev.parent->fwnode)); ``` `drm_dp_hpd_bridge_register()` returns a `struct device *` which can be an ERR_PTR. The return value is silently ignored. While there may not be much that can be done from a bus notifier, at least a `dev_warn()` would help debugging. **Issue 2: Fragile device hierarchy assumption** The comment documents the parent traversal (`alt->dev.parent->parent` = USB-C controller device), but this is a fragile assumption about the device hierarchy. If the hierarchy ever changes, this will silently break. Consider whether there's a more robust API to get the controller device. **Issue 3: Module exit doesn't clean up bridges** `drm_aux_hpd_typec_dp_bridge_module_exit()` only unregisters the notifier but doesn't clean up any bridges that were registered via `drm_dp_hpd_bridge_register()`. Those are `devm`-managed on the controller device, so they'll survive module unload. This may be intentional, but it means loading/unloading this module repeatedly could have unexpected interactions. **Minor: Missing MODULE_AUTHOR** --- Generated by Claude Code Patch Reviewer