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: tc358762: Drop drm_bridge_funcs.mode_set Date: Sat, 16 May 2026 12:04:24 +1000 Message-ID: In-Reply-To: <20260513-tc358762-fixes-v3-11-6698b55008b9@ideasonboard.com> References: <20260513-tc358762-fixes-v3-0-6698b55008b9@ideasonboard.com> <20260513-tc358762-fixes-v3-11-6698b55008b9@ideasonboard.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 Replaces the deprecated `.mode_set` callback (which stored the mode into `c= tx->mode`) with fetching the mode from atomic state in `tc358762_enable()`.= Uses `drm_atomic_get_new_connector_for_encoder()` =E2=86=92 `drm_atomic_ge= t_new_connector_state()` =E2=86=92 `drm_atomic_get_new_crtc_state()` =E2=86= =92 `crtc_state->mode`. **Issue: Missing NULL checks.** The return values of `drm_atomic_get_new_co= nnector_for_encoder()`, `drm_atomic_get_new_connector_state()`, and `drm_at= omic_get_new_crtc_state()` can all be NULL. While in practice they shouldn'= t be NULL when called from `atomic_enable` (the CRTC/connector must be acti= ve), other bridge drivers (e.g., `dw-dp.c`, `adv7511_drv.c`, `analogix_dp_c= ore.c`) do NULL-check the connector return. The sibling driver `tc358768.c`= doesn't check either, but that doesn't make it right. ```c connector =3D drm_atomic_get_new_connector_for_encoder(state, bridge->encod= er); conn_state =3D drm_atomic_get_new_connector_state(state, connector); crtc_state =3D drm_atomic_get_new_crtc_state(state, conn_state->crtc); mode =3D &crtc_state->mode; ``` A NULL `connector` here would cause a NULL-pointer dereference on the next = line. Recommend at minimum a `WARN_ON(!connector)` guard with early return,= consistent with the pattern in `adv7511_drv.c`. Also uses `crtc_state->mode` rather than `crtc_state->adjusted_mode`. This = is consistent with the original `.mode_set` which was passed the unadjusted= mode, so it preserves behavior. But for a bridge that needs actual hardwar= e timings, `adjusted_mode` is often more appropriate. Worth considering whe= ther `adjusted_mode` would be better, though for this simple bridge the dif= ference is likely negligible. --- Generated by Claude Code Patch Reviewer