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 21:30:31 +1000 Message-ID: In-Reply-To: <20260520160744.130841-2-tzimmermann@suse.de> References: <20260520160744.130841-1-tzimmermann@suse.de> <20260520160744.130841-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 **Correctness**: The new `fb_set_var_from_user()` correctly consolidates th= e three-step pattern: check with `fbcon_modechange_possible()`, call `fb_se= t_var()`, then `fbcon_update_vcs()`. This is a clean factoring. **Behavioral fix for sysfs path**: The `activate()` function in `fbsysfs.c`= previously skipped `fbcon_modechange_possible()`. Now it calls it via the = helper. This is a fix for an existing inconsistency, as the commit message = notes =E2=80=94 good. **Behavioral change in sh_mobile_lcdcfb**: The original code at `sh_mobile_= lcdcfb.c:1775`: ```c var.activate =3D FB_ACTIVATE_NOW; ... fbcon_update_vcs(info, true); ``` becomes (via `fb_set_var_from_user`): ```c fbcon_update_vcs(info, var->activate & FB_ACTIVATE_ALL); ``` Since `FB_ACTIVATE_NOW` is 0 and `FB_ACTIVATE_ALL` is 64, the `all` paramet= er changes from `true` to `false`. This means only the current VC gets upda= ted rather than all VCs. This may be the intended fix (the cover letter say= s "fix inconsistencies among the various code paths"), but it's a user-visi= ble behavioral change that should be explicitly mentioned in the commit mes= sage. **EXPORT_SYMBOL**: `fb_set_var_from_user` is exported with `EXPORT_SYMBOL` = (not `_GPL`), matching `fb_set_var`. Fine since it's used by `ps3fb` and `s= h_mobile_lcdcfb` which can be modules. **Minor**: The removed `EXPORT_SYMBOL(fbcon_update_vcs)` and `EXPORT_SYMBOL= _GPL(fbcon_modechange_possible)` in fbcon.c are now unnecessary since only = fbmem.c calls them =E2=80=94 this is correct. --- Generated by Claude Code Patch Reviewer