From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: Initial support Cadence MHDP8501(HDMI/DP) for i.MX8MQ Date: Sun, 12 Apr 2026 13:34:44 +1000 Message-ID: In-Reply-To: <20260407-dcss-hdmi-upstreaming-v21-0-4681070ab82f@oss.nxp.com> References: <20260407-dcss-hdmi-upstreaming-v21-0-4681070ab82f@oss.nxp.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Overall Series Review Subject: Initial support Cadence MHDP8501(HDMI/DP) for i.MX8MQ Author: Laurentiu Palcu Patches: 14 Reviewed: 2026-04-12T13:34:44.694971 --- This v21 series adds initial support for the Cadence MHDP8501 HDMI/DP bridge and HDP-TX PHY for NXP i.MX8MQ. It is a long-running series (21 revisions) that refactors the existing mhdp8546 mailbox code into a shared helper module, adds a new bridge driver, PHY driver, dt-bindings, and DTS entries. **Structural assessment:** The series is reasonably well-organized: shared helpers first, then mhdp8546 refactoring, followed by bindings, the new bridge driver, PHY bindings/driver, and finally DTS. The code quality is generally acceptable for a v21, having gone through significant review, but several issues remain that need attention before merging. **Key concerns:** 1. **Critical behavioral change in patch 2**: The `cdns_mhdp_reg_write()` helper changes the mailbox command from `DPTX_WRITE_REGISTER` (DP_TX module) to `GENERAL_REGISTER_WRITE` (GENERAL module), which changes the firmware command semantics for the existing mhdp8546 driver. 2. **Global mutex design**: A process-global static mutex for mailbox access means multiple independent MHDP instances on different SoCs would serialize against each other, though this is unlikely on i.MX8MQ. 3. **DTS address overlap**: Both PHY and bridge nodes claim the same `reg = <0x32c00000 0x100000>` without sub-partitioning. While this works because `devm_ioremap` doesn't conflict with itself, it's unusual and deserves a comment. 4. **Missing `cdns,bridge-type` from the SoC dtsi**: This vendor property in the board DTS is a design smell. --- --- Generated by Claude Code Patch Reviewer