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/rockchip: analogix_dp: Add support for RK3576 Date: Tue, 26 May 2026 07:34:42 +1000 Message-ID: In-Reply-To: <20260525082033.117569-11-damon.ding@rock-chips.com> References: <20260525082033.117569-1-damon.ding@rock-chips.com> <20260525082033.117569-11-damon.ding@rock-chips.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review Adds the platform-specific chip data and OF match entry: ```c +static const struct rockchip_dp_chip_data rk3576_edp[] = { + { + .chip_type = RK3576_EDP, + .reg = 0x27dc0000, + }, + { /* sentinel */ } +}; ``` **Key observation:** RK3576's chip data does not set `edp_mode` or `lcdc_sel` GRF fields, unlike RK3588 and the older SoCs. I verified that `rockchip_grf_field_write()` checks `!field->valid` and returns 0 early (at `analogix_dp-rockchip.c:107`), so the zero-initialized `edp_mode` field is safe -- the GRF write in `rockchip_dp_poweron()` and `rockchip_dp_powerdown()` will be a no-op. This implies RK3576 doesn't need an eDP mode GRF switch, which is plausible if the VO subsystem handles muxing differently. Similarly, no `lcdc_sel` means the CRTC selection path in `rockchip_dp_drm_encoder_enable()` will skip the GRF write. This should be fine if RK3576 has a fixed eDP-to-CRTC mapping (no mux). The OF table entry is correctly ordered alphabetically. The sentinel terminator is present. **Overall:** This is a clean, well-organized series at v7 maturity. The RK3588 clock fix (patches 1-5) is a proper standalone fix with Fixes tags. The RK3576 support (patches 6-10) reuses the proven RK3588 code path with minimal additions. No correctness bugs found. The series is ready to merge. --- Generated by Claude Code Patch Reviewer