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/msm: fbdev: Inline msm_alloc_stolen_fb() Date: Thu, 23 Apr 2026 08:30:53 +1000 Message-ID: In-Reply-To: <20260421125733.209568-3-tzimmermann@suse.de> References: <20260421125733.209568-1-tzimmermann@suse.de> <20260421125733.209568-3-tzimmermann@suse.de> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Status: Looks good, minor observation.** The inlining is done correctly. The key changes are: - The error handling for the non-stolen BO fallback is now properly nested = (`if (IS_ERR(bo))` inside the first `IS_ERR(bo)` block), which is a slight = improvement over the original's sequential checks. - `mode_cmd.modifier[0] =3D DRM_FORMAT_MOD_LINEAR` is explicitly set, which= is a no-op since `DRM_FORMAT_MOD_LINEAR` is 0 and the struct is zero-initi= alized. The commit message correctly notes this. - The error cleanup path is improved with descriptive labels (`err_drm_fram= ebuffer_remove`, `err_drm_gem_object_put`). One observation: the `drm_gem_object_put(bo)` at `err_drm_gem_object_put` r= uns even on `drm_framebuffer_remove` errors. In the old code, `msm_framebuf= fer_init()` failure was handled by the function itself (it would `drm_gem_o= bject_put` internally on error), and then the caller would also `ERR_CAST` = without another put. Now the new code does `goto err_drm_gem_object_put` wh= ich adds an explicit put. This is correct because `msm_framebuffer_init()` = takes ownership of `bos[]` on success but does NOT consume the reference on= failure =E2=80=94 the caller's `bo` still needs to be put. The existing `m= sm_alloc_stolen_fb()` had the same pattern at its line: ```c drm_gem_object_put(bo); ``` so this is faithful to the original. --- Generated by Claude Code Patch Reviewer