From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: fbdev: Wrap user-invoked calls to fb_set_var() in helper Date: Mon, 25 May 2026 18:52:07 +1000 Message-ID: In-Reply-To: <20260522123019.211059-2-tzimmermann@suse.de> References: <20260522123019.211059-1-tzimmermann@suse.de> <20260522123019.211059-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 Creates `fb_set_var_from_user()` in `fbmem.c` that bundles `fbcon_modechang= e_possible()` + `fb_set_var()` + `fbcon_update_vcs()` into a single call, t= hen converts all callers. The chrdev ioctl path (`fb_chrdev.c`) was already doing all three steps =E2= =80=94 this is a direct refactor with no behavior change. The sysfs `activate()` path (`fbsysfs.c`) was missing the `fbcon_modechange= _possible()` check. The new helper adds it, which is a correctness improvem= ent =E2=80=94 mode changes through sysfs will now be properly validated aga= inst fbcon constraints. The `ps3fb` driver was also missing the `fbcon_modechange_possible()` check= , which is similarly a correctness improvement. **Semantic change in `sh_mobile_lcdcfb`:** The original code hardcoded `fbc= on_update_vcs(info, true)` (update all VCs). The new helper passes `var->ac= tivate & FB_ACTIVATE_ALL`. Since `sh_mobile_fb_reconfig()` sets `var.activa= te =3D FB_ACTIVATE_NOW` (which is 0), `FB_ACTIVATE_NOW & FB_ACTIVATE_ALL` e= valuates to `false`, meaning only the foreground VC gets updated instead of= all VCs. This is a functional change =E2=80=94 whether it's intentional or= a regression depends on the driver's requirements. It's worth documenting = in the commit message if intentional, or fixing if not. The removal of `EXPORT_SYMBOL` for `fbcon_update_vcs` and `EXPORT_SYMBOL_GP= L` for `fbcon_modechange_possible` is correct since patches 1-3 eliminate a= ll out-of-module callers before patch 4 fully internalizes them. The new `fb_set_var_from_user` is `EXPORT_SYMBOL` which is appropriate sinc= e `ps3fb` and `sh_mobile_lcdcfb` are separate drivers that need it. --- Generated by Claude Code Patch Reviewer