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: cadence: Update mhdp8546 mailbox access functions Date: Sun, 12 Apr 2026 13:34:45 +1000 Message-ID: In-Reply-To: <20260407-dcss-hdmi-upstreaming-v21-2-4681070ab82f@oss.nxp.com> References: <20260407-dcss-hdmi-upstreaming-v21-0-4681070ab82f@oss.nxp.com> <20260407-dcss-hdmi-upstreaming-v21-2-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 Patch Review **Critical: `cdns_mhdp_reg_write()` behavioral change.** The old mhdp8546 `cdns_mhdp_reg_write()` used: ```c cdns_mhdp_mailbox_send(mhdp, MB_MODULE_ID_DP_TX, DPTX_WRITE_REGISTER, sizeof(msg), msg); ``` with a 16-bit address in a 6-byte message. The new helper uses: ```c cdns_mhdp_mailbox_send(base, MB_MODULE_ID_GENERAL, GENERAL_REGISTER_WRITE, sizeof(msg), msg); ``` with a 32-bit address in an 8-byte message. This changes the firmware command path from DP_TX to GENERAL module. While both may ultimately write to the same register space, the firmware could interpret them differently. **This needs explicit confirmation that the GENERAL_REGISTER_WRITE command is functionally equivalent for all the registers the mhdp8546 driver writes to.** This could cause a silent regression on existing mhdp8546 platforms. **Similarly**, `cdns_mhdp_reg_read()` changed from a local function that directly calls send/recv_header/recv_data to using the new `cdns_mhdp_mailbox_send_recv()` helper. The old code used `MB_MODULE_ID_GENERAL` already, so this is fine. **Good**: The `cdns_mhdp_base` initialization in probe is clean: ```c mhdp->base.dev = mhdp->dev; mhdp->base.regs = mhdp->regs; mhdp->base.sapb_regs = mhdp->sapb_regs; ``` **Alignment nit:** Several `cdns_mhdp_mailbox_send_recv()` calls have inconsistent indentation of continuation arguments, e.g.: ```c ret = cdns_mhdp_mailbox_send_recv(&mhdp->base, MB_MODULE_ID_GENERAL, GENERAL_MAIN_CONTROL, ``` The second line should align under `&mhdp->base`. --- Generated by Claude Code Patch Reviewer