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: Enable hclk for RK3588 Date: Sat, 16 May 2026 12:26:17 +1000 Message-ID: In-Reply-To: <20260513074414.2053435-6-damon.ding@rock-chips.com> References: <20260513074414.2053435-1-damon.ding@rock-chips.com> <20260513074414.2053435-6-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 **Status: Looks good, one observation** Uses `devm_clk_get_optional_enabled()` to fetch and enable the `"hclk"` clock: ```c clk = devm_clk_get_optional_enabled(dev, "hclk"); if (IS_ERR(clk)) return dev_err_probe(dev, PTR_ERR(clk), "failed to get hclk property\n"); ``` Using `optional` is correct for backward compatibility with older device trees that don't include this clock. Using `devm_clk_get_optional_enabled` means the clock stays enabled from probe until device removal -- this mirrors the previous implicit behavior where the GRF phandle reference kept the bus clock active. Since this is a bus clock needed for register access, keeping it always-on is reasonable. The local `struct clk *clk` variable is intentionally unused after the check since the devm-managed enabled clock needs no further management -- this is fine. --- Generated by Claude Code Patch Reviewer