From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: clk: bcm: rpi: Manage clock rate in prepare/unprepare callbacks Date: Fri, 13 Mar 2026 13:47:07 +1000 Message-ID: In-Reply-To: <20260312-v3d-power-management-v7-1-9f006a1d4c55@igalia.com> References: <20260312-v3d-power-management-v7-0-9f006a1d4c55@igalia.com> <20260312-v3d-power-management-v7-1-9f006a1d4c55@igalia.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review This patch moves V3D clock rate management into the clock framework's prepare/unprepare callbacks, so the clock rate is set to minimum on unprepare (achieving actual power savings despite firmware limitations) and restored to maximum on prepare for clocks marked with `maximize`. The approach is well-justified with good comments explaining the firmware workaround. Two observations: 1. In `raspberrypi_fw_unprepare()`, the return value of `raspberrypi_fw_set_rate()` is ignored: ```c raspberrypi_fw_set_rate(hw, min_rate, 0); ``` This is acceptable since unprepare is a void function and the clock is being disabled anyway, but it might be worth a comment or at least a `WARN_ON` for debugging. 2. In `raspberrypi_fw_prepare()`, the error from `raspberrypi_fw_set_rate()` is propagated through `ret`, but if the clock was already successfully enabled, a failed rate set will return an error without undoing the clock enable. This could leave the clock enabled but at the wrong rate, with the caller thinking prepare failed. Consider either undoing the clock enable on rate-set failure, or treating a rate-set failure as non-fatal. The removal of `variant->min_rate = max_rate` in `raspberrypi_clk_register()` cleanly shifts this responsibility to the prepare callback. --- Generated by Claude Code Patch Reviewer