From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: dt-bindings: display: rockchip: analogix-dp: Fix hclk as third clock for RK3588 Date: Tue, 26 May 2026 07:34:40 +1000 Message-ID: In-Reply-To: <20260525082033.117569-2-damon.ding@rock-chips.com> References: <20260525082033.117569-1-damon.ding@rock-chips.com> <20260525082033.117569-2-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 Good approach. The binding change allows `hclk` as an alternative third clock name alongside `grf`, and then uses per-compatible `allOf` conditionals to constrain clock counts: ```yaml - enum: - grf - hclk ``` The per-compatible constraints look correct: - RK3288: maxItems 2 (no third clock) - RK3399: minItems 3 (requires `grf`) - RK3588: minItems 3 (requires `hclk`) **Minor observation:** The binding doesn't enforce *which* third clock name each SoC must use -- both `grf` and `hclk` are valid for all 3-clock SoCs. The RK3399 `allOf` block requires 3 clocks but doesn't constrain the third clock-name to `grf` specifically, and similarly RK3588 doesn't constrain it to `hclk`. This works because in practice DTS authors follow the convention, and the top-level enum restricts it to only these two valid names. Acceptable as-is, but a DTS could technically declare RK3399 with `hclk` and pass validation. The Fixes tag is appropriate since this corrects the clock representation for RK3588. --- Generated by Claude Code Patch Reviewer