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/vblank: Add drmm_vblank_init() to indicate managed cleanup Date: Thu, 28 May 2026 12:19:14 +1000 Message-ID: In-Reply-To: <20260527133917.207150-2-tzimmermann@suse.de> References: <20260527133917.207150-1-tzimmermann@suse.de> <20260527133917.207150-2-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 This patch renames `drm_vblank_init()` to `drmm_vblank_init()` to follow th= e DRM managed-resource naming convention, adds a `flags` argument, and keep= s the old name as a deprecated inline wrapper. **Observations:** - The rename is appropriate =E2=80=94 `drm_vblank_init()` already uses `drm= m_add_action_or_reset()` internally, so the `drmm_` prefix is the correct c= onvention. - The new `flags` field is added to `struct drm_vblank_crtc` without docume= ntation: ```c + unsigned int flags; }; ``` Given that every other field in this struct has a kerneldoc `/** @field: = ... */` comment, adding one here (even brief) would be consistent. Somethin= g like `/** @flags: flags for vblank handling (e.g., DRM_VBLANK_FLAG_SIMULA= TED) */`. - The internal release callback rename from `drm_vblank_init_release` to `d= rmm_vblank_init_release` is a nice consistency touch. - The inline backward-compat wrapper passing `0` for flags is clean: ```c +static inline int drm_vblank_init(struct drm_device *dev, unsigned int n= um_crtcs) +{ + return drmm_vblank_init(dev, num_crtcs, 0); +} ``` This avoids a tree-wide conversion in one go. - Minor: the `flags` parameter is not validated in any way (e.g., rejecting= unknown bits). This is fine for an internal kernel API, but if flags grow = in the future, you might want a `if (flags & ~DRM_VBLANK_FLAG_SIMULATED) re= turn -EINVAL;` check. Not required now. --- Generated by Claude Code Patch Reviewer