From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: soc: cadence: Create helper functions for Cadence MHDP Date: Mon, 25 May 2026 22:48:48 +1000 Message-ID: In-Reply-To: <20260519-dcss-hdmi-upstreaming-v23-1-5615524a9c63@oss.nxp.com> References: <20260519-dcss-hdmi-upstreaming-v23-0-5615524a9c63@oss.nxp.com> <20260519-dcss-hdmi-upstreaming-v23-1-5615524a9c63@oss.nxp.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review This patch extracts mailbox access code from the mhdp8546 driver into a new= shared helper under `drivers/soc/cadence/`. The design is clean with per-i= nstance `mailbox_mutex`, proper `lockdep_assert_held()` annotations, and ad= dress validation on DPCD/register reads. **Issues:** 1. **Commit message typo**: "helper fucntions" and "MDHP8501" should be "he= lper functions" and "MHDP8501". 2. **HDCP receiver ID buffer overflow risk** in `cdns_mhdp_secure_mailbox_s= end_recv_multi()`: ```c if (module_id =3D=3D MB_MODULE_ID_HDCP_TX && opcode =3D=3D HDCP_TRAN_IS_REC_ID_VALID) ret =3D mhdp_mailbox_recv_data(base, resp2, 5 * resp1[0], true); ``` `resp1[0]` is firmware-controlled. If it returns a value larger than the bu= ffer behind `resp2`, this overflows. The caller in mhdp8546-hdcp.c passes `= 0` for `resp2_size` with `hdcp_rx_id` buffer =E2=80=94 but the size of that= buffer is `HDCP_MAX_RECEIVERS * 5 =3D 160` bytes. There's no validation th= at `resp1[0] <=3D HDCP_MAX_RECEIVERS` here. 3. **Code duplication**: The non-secure and secure variants of `cdns_mhdp_m= ailbox_send_recv` and `cdns_mhdp_mailbox_send_recv_multi` are nearly identi= cal except for the `secure` boolean. The internal functions already take a = `bool secure` parameter =E2=80=94 consider whether the six exported wrapper= s could be reduced to three with a boolean parameter, or if the current app= roach is intentional for API clarity. 4. **Placement under `drivers/soc/`**: This is a display/PHY helper library= . While it provides mailbox access for an IP block, placing it under `drive= rs/soc/cadence/` may be debatable. The DRM bridge subsystem maintainers or = the linux-soc list should weigh in on whether this belongs under `drivers/s= oc/` or perhaps `drivers/firmware/` or `drivers/mfd/`. --- Generated by Claude Code Patch Reviewer